refactor!: Use track container in Core `CKF` and pass track proxies to delegates
Wires tracks through Core CKF to track branches and passes those tracks into the branch stopper delegate. This allows the user to count experiment specific track quantities on those branches and then cut on those (e.g. pixel holes).
blocked by
- https://github.com/acts-project/acts/pull/3187
📊: Physics performance monitoring for 1f43ac2f071695f1686c26de693c34587789755a
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
@timadye this should allow us to count experiment specific stopping criteria inside the branch stopper without adding state to it but rather store this information on the branches
no significant performance changes measured
- This seems a big change adding the track container just to be able to access the track proxy from the branch stopper. Is this the only benefit, or is it useful in the other delegates or for another reason?
Right I also think this is a big change but there are other benefits from that:
- The branches in the CKF are now modeled as
tracksin the track container. Instead of havingactiveTipswhich was only valid after we created a track state we can now talk aboutbracheswhich do not necessarily have to have track states. - Because of that we can also attach the extrapolated track parameters at the target surface to the track within the CKF which was previously not possible. I exercised that in https://github.com/acts-project/acts/pull/3195.
- I had previously hoped that we could use the track state to count experiment specific track quantities as they were visited. Is this not possible?
This is possible by adding custom columns to the track container and counting the stats within the branch stopper now. Although I didn't exercise that yet.
- Before introducing such a change, it would be good to check that it actually can do what it claims to. Would it be possible to have an example in the ActsExamples branch stopper? An actually functional example might be to cut on the number of 1D or 2D measurements.
I agree, I will try to make some reasonable showcase in the Examples. I am not sure if we will be able to merge that but I guess it would be already valuable for just pointing at some code.
Codecov Report
Attention: Patch coverage is 29.33333% with 53 lines in your changes missing coverage. Please review.
Project coverage is 47.66%. Comparing base (
cf9d872) to head (6096e56). Report is 21 commits behind head on main.
:exclamation: Current head 6096e56 differs from pull request most recent head ffdf2cc
Please upload reports for the commit ffdf2cc to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## main #3161 +/- ##
==========================================
+ Coverage 47.24% 47.66% +0.42%
==========================================
Files 508 509 +1
Lines 30041 29422 -619
Branches 14586 14131 -455
==========================================
- Hits 14192 14024 -168
+ Misses 5375 5282 -93
+ Partials 10474 10116 -358
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I fear the coverage will not be easy to recover because the new ifs handle an edge case where we reject the first measurement and potentially more and finally maybe all of them. Do you think we can override in this case @paulgessinger ?
@andiwand I was just playing around with the setting for now. No need to overcomplicate our process right now. I updated the setting now (to 25%) and it should be updated when the CI runs the next time.
@timadye one thing I noticed is that we have a hole cut now in the track selector and in the CkfConfig. I guess it is okay for the Examples for now because aligning this looks complicated
yeah, and the pix/str hole cuts aren't eta-dependent. Unfortunately the Acts::TrackSelector is a core class, so we can't add detector-dependent selections to it. I think that could be improved, but that is a whole other thing.
This has some output changes, and the ExaTrkX job seems to fail to build. Will you look into this @andiwand?
I will have a look. I think the output changes appeared with https://github.com/acts-project/acts/pull/3161/commits/255906dbd1fc3ed5f8af2d0b374eb1370bc300ac and exatrkx failed for a while but I didn't realize it. I guess I just have to absorb the braking change there
@andiwand I'm just a bit confused why that would be. Does it make sense to you?
The output changed again after I refactored the tip state counting to use the track interface. Looks already way healthier than before. I don't know exactly what causes the difference now but my hunch is that the branch stopper behaves a little bit differently than before.
Looking at the track classification
Which shows a reduction in duplicate tracks which also affects all the other plots to change.
@andiwand physmon changes largely don't seem to be very large.
I will update the reference
:white_check_mark: Athena integration test results [6643b83069296d264cc8d70a1315275220037cbb]
:white_check_mark: All tests successful
I executed test_ActsWorkflow manually with main--ACTS and it seems to work
So we think it's just the inconsistency?
I think so - I triggered another run with the modified CI setup. Lets see if that goes through. Otherwise I would suggest to merge these PRs at the end of the day to get them into the nightly and then fix potential problems afterwards.
Athena is happy now
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
44.3% Coverage on New Code
0.0% Duplication on New Code