acts
acts copied to clipboard
acts-project
feat: Two-way CKF example
Resurrect #2717 from @andiwand, updating to the latest main, and removing earlier test (MyTrackFindingAlgorithm).
blocked by
- https://github.com/acts-project/acts/pull/3074
- https://github.com/acts-project/acts/pull/3075
- https://github.com/acts-project/acts/pull/3076
- https://github.com/acts-project/acts/pull/3077
- https://github.com/acts-project/acts/pull/3078
- https://github.com/acts-project/acts/pull/3086
Codecov Report
Attention: Patch coverage is 25.00000% with 9 lines in your changes are missing coverage. Please review.
Project coverage is 49.02%. Comparing base (
3203649) to head (3d3c37f). Report is 5 commits behind head on main.
:exclamation: Current head 3d3c37f differs from pull request most recent head d9f2302. Consider uploading reports for the commit d9f2302 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## main #3066 +/- ##
==========================================
- Coverage 49.09% 49.02% -0.07%
==========================================
Files 497 494 -3
Lines 29155 29061 -94
Branches 13851 13798 -53
==========================================
- Hits 14314 14248 -66
- Misses 4908 4930 +22
+ Partials 9933 9883 -50
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
📊: Physics performance monitoring for d9f23023a73d7b262678c3a39f5aa1a9a3ff585d
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
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Currently this has the default twoWay = true (in TrackFindingAlgorithm.hpp and reconstruction.py). Should we make it false to keep the old behaviour, or make this the new default?
Started breaking things off
- https://github.com/acts-project/acts/pull/3073
- https://github.com/acts-project/acts/pull/3074
- https://github.com/acts-project/acts/pull/3075
I think this now works as it should. The only thing which is a bit annoying is the second extrapolation which would not be necessary if the CKF would spit out parameters at the target surface.
But it might be good enough for now and we improve the Core CKF in the next iteration.
What do you think @timadye ?
The changes look OK to me.
Unfortunately the results aren't so good running on ITk with ttbar PU200.
- It crashes after 81 events with error:
/home/ppd/adye/acts/build/src/Core/include/Acts/Utilities/VectorHelpers.hpp:142: std::array<double, 4> Acts::VectorHelpers::evaluateTrigonomics(const Acts::Vector3&): Assertion `sinTheta != 0 && "VectorHelpers: Vector is parallel to the z-axis " "which leads to division by zero"' failed.
- It has a lot more
ERRORs. Here's a count of the most common:
284888 TrackFinding ERROR Smoothing track 1 failed with error KalmanFitterError:3
284888 TrackFinding ERROR First smoothing for seed 22 and track 1 failed with error KalmanFitterError:3
140630 TrackFinding ERROR no intersection found
140630 TrackFinding ERROR failed to find track state for extrapolation
74918 TrackFinding ERROR Extrapolation for seed 14 and track 0 failed with error TrackExtrapolationError:2
65712 TrackFinding ERROR Second extrapolation for seed 14 and track 6 failed with error TrackExtrapolationError:2
8238 TrackFinding ERROR Propagation reached the step count limit of 1000 (did 1000 steps)
- If I set
twoWay=false, it seems to hang on event 97. All other events finished, but an hour later one thread is still running with 100% CPU, and all the other threads are idle. The 8-thread process is using 64.6GB RAM. - With
twoWay=false, I still get quite a lot ofERRORs, but this is only about twice as many as we had before #2722, albeit different messages:
94977 TrackFinding ERROR no intersection found
94977 TrackFinding ERROR failed to find track state for extrapolation
94977 TrackFinding ERROR Extrapolation for seed 2 and track 0 failed with error TrackExtrapolationError:2
Since it misbehaves even with twoWay=false, I think there is still a problem with other core changes, notably #2722. So maybe not specific to this PR, but needs fixing before we can see the performance implications, at least for the ITk.
Thank you for the detailed investigation @timadye !
I will look into this next week.
Just checked the latest version with the ITk, including restore first track patch. The number of ERROR messages are now similar between runs with twoWay=true and false. I still get a crash and hang as reported above.
I think that means we still have a problem running with the ITk, but it doesn't seem to be specifically associated with this PR.
Made a quick performance comparison between https://github.com/acts-project/acts/pull/3066/commits/de9a0c915b3a7e994ac1d948593af05b23324e89 and current (main and changed respectively in the plot)
ttbar PU 200 fatras default seeding
Looks like the additional track copies cost about 3%
@timadye
:red_circle: Athena integration test results [d1f4939d6a940614184ebd590986cdf6aa73be20]
:red_circle: Some tests have failed!
Please investigate the pipeline!
I ran the ITk ttbar+PU200 tests again (this is standalone ACTS). This still crashed (twoWay=true) or went into a loop (twoWay=false) within 100 events. As suggested before, this is probably a result of #2722 and only gives different symptoms with this PR.