fix: More precise track parameter estimation for high curvature tracks

[beomki-yeo]

Calculation from #2211 without approximation arcsin.

I will put some plots later to estimate how correct the PR is compared to current calculation

Following is the plot for residual between track parameter estimation and truth value (for p and qop separately)

I tested with

• three helix points generated at 200, 400, 600 mm of trajectory
• the initial track direction in [1/sqrt(2),0,1/sqrt(2)],
• B field: 2T in z-axis
• Repeat the same test from 0.1 GeV/c , 0.2 GeV/c , ... , 9.9 GeV/c

My impression is that the original track parameter estimation is not as bad as I imagined before this study. So I am not sure if we will want to proceed with the change eventually.

[codecov]

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 48.86%. Comparing base (6507d84) to head (4df7b6c).

Files	Patch %	Lines
...clude/Acts/Seeding/EstimateTrackParamsFromSeed.hpp	25.00%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2982   +/-   ##
=======================================
  Coverage   48.86%   48.86%           
=======================================
  Files         493      493           
  Lines       29058    29058           
  Branches    13798    13798           
=======================================
  Hits        14200    14200           
  Misses       4962     4962           
  Partials     9896     9896           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

📊: Physics performance monitoring for 4df7b6c

Full contents

physmon summary

• ✅ CKF truth_smeared
• ✅ IVF truth_smeared
• ✅ AMVF truth_smeared
• ✅ Track Summary CKF truth_smeared
• ✅ Seeding truth_estimated
• ✅ CKF truth_estimated
• ✅ IVF truth_estimated
• ✅ AMVF truth_estimated
• ✅ Track Summary CKF truth_estimated
• ✅ Seeding seeded
• ✅ CKF seeded
• ✅ IVF seeded
• ✅ AMVF seeded
• ✅ AMVF (+grid seeder) seeded
• ✅ Track Summary CKF seeded
• ✅ Seeding orthogonal
• ✅ CKF orthogonal
• ✅ IVF orthogonal
• ✅ AMVF orthogonal
• ✅ Track Summary CKF orthogonal
• ✅ Ambisolver seeded
• ✅ Ambisolver orthogonal
• ✅ Seeding ttbar
• ✅ CKF ttbar
• ✅ Ambisolver
• ✅ Track Summary CKF ttbar
• ✅ AMVF ttbar
• ✅ AMVF (+grid seeder) ttbar
• ✅ Truth tracking (GSF)
• ✅ Truth tracking
• ✅ Truth tracking (GX2F)
• ✅ Particles fatras
• ✅ Particles geant4

[andiwand]

do you know if this has any CPU performance implications?

[beomki-yeo]

I don't know the number exactly. I believe the performance won't change a lot because it is just having one more line with arcsin per track.

[AJPfleger]

The math seems to be correct and I also don't see a need to look deeper in the performance change from the asin.

Could you update the hashes?

[andiwand]

@CarloVarni did some optimizations on this #2699

[CarloVarni]

From my side there are no objection to this. If this provides a more precise computation w.r.t. the current implementation, then we should go for it. I'm a little surprised @beomki-yeo that there are no changes in physics performance, I would have expected (minor) changes in the CKF output.

On the CPU impact, the estimation of the parameter from seeds is already very cheap. Even if there is a small increase due to this change that would be hardly relevant in the entire track reconstruction process. So I'd say we can go on with this.

[andiwand]

looking at the performance monitoring it seems that there are some changes but below the failure threshold

I guess this is expected since the KF will get the initial params with a big covariance and estimate almost from scratch

[CarloVarni]

ok thanks @andiwand then I'm approving this

[andiwand]

@beomki-yeo can you update the hashes? and @AJPfleger needs to lower the red flag after this

[beomki-yeo]

@CarloVarni @AJPfleger @andiwand Thanks for the comments and sorry that I didn't reply for long time.
Yeah I think I am also onboard with the changes based on the comments XD

[acts-project-service]

🔴 Athena integration test results [28c3133]

🔴 Some tests have failed!

Please investigate the pipeline!

status	job	report
🔴	run_unit_tests
🟢	test_ActsEFTrackFit
🟢	test_ActsBenchmarkWithSpot
🟢	test_ActsConversionWorkflow
🟢	test_ActsWorkflow
🟢	test_ActsValidateAmbiguityResolution
🟢	test_ActsValidateResolvedTracks
🟢	test_ActsValidateTracks
🟢	test_ActsValidateActsCoreSpacePoints
🟢	test_ActsValidateActsSpacePoints
🟢	test_ActsValidateSeeds
🟢	test_ActsValidateOrthogonalSeeds
🟢	test_ActsValidateClusters
🟢	test_ActsPersistifyEDM
🟢	test_ActsGSFRefitting
🟢	test_ActsKfRefitting
🟢	test_ActsExtrapolationAlgTest
🟢	test_ActsITkTest
🟢	run_workflow_tests_run4_mc
🟢	run_workflow_tests_run2_mc
🟢	run_workflow_tests_run2_data
🟢	run_workflow_tests_run3_mc
🟢	run_workflow_tests_run3_data
🟢	run_art_test: test_data18_13TeV_1000evt
🟢	run_art_test: test_ttbarPU40_reco

[paulgessinger]

The Athena output change seems to be related to this. We seem to be getting a single digit number of additional tracks. I think we can tolerate this.

(cc @CarloVarni)