-
Notifications
You must be signed in to change notification settings - Fork 157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Fix CKF finalization #2299
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2299 +/- ##
=======================================
Coverage 49.27% 49.28%
=======================================
Files 450 450
Lines 25407 25405 -2
Branches 11728 11728
=======================================
Hits 12520 12520
+ Misses 4549 4547 -2
Partials 8338 8338
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@timadye if you could test this and/or have a look at the changes it would be great! |
Sure, let me update my build and I'll give it a go. I'm hoping for great things! 😁 |
📊 Physics performance monitoring for 897d215Summary VertexingSeedingCKFAmbiguity resolutionTruth tracking (Kalman Filter)Truth tracking (GSF) |
Hi @andiwand, This is a HUGE improvement! I tested with
Apart from the increase in CPU time, this is all extremely positive. I hope this fix can be merged. Thanks, |
sounds great! it might be that the remaining errors are fixed by #2300 if those are coming from Fatras |
one silly question: how is it this PR is not changing performances in the tests but it is for the ITk script? What am I missing? |
we do not have a ttbar event in the physmon but only single particles which seem not to be affected |
oh right, I forgot that paul's PR that adds ttbar test is not in yet. Thanks @andiwand . I'd say we can proceed with this then |
Apparently our CKF was not able to smooth multiple tracks for a single seed in some cases. The problem was that we expected the CKF actor to be called a second time after the first smoothing was done but there is a Stepper step in between which could kick you out of the detector because the step length is basically unconstrained. fixes - acts-project#1484 --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Apparently our CKF was not able to smooth multiple tracks for a single seed in some cases. The problem was that we expected the CKF actor to be called a second time after the first smoothing was done but there is a Stepper step in between which could kick you out of the detector because the step length is basically unconstrained.
fixes