-
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
feat: change the behaviour of enableCutsForSortedSP in seedFinder #1213
feat: change the behaviour of enableCutsForSortedSP in seedFinder #1213
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1213 +/- ##
==========================================
+ Coverage 47.94% 47.97% +0.02%
==========================================
Files 373 373
Lines 19495 19484 -11
Branches 9152 9148 -4
==========================================
Hits 9347 9347
+ Misses 3817 3806 -11
Partials 6331 6331
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
…lipeCoelho/acts into enable-cuts-for-sorted-sp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this is separate now, why would one want to sort the SP if the sorting is not used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much everyone for the collaboration on this, I think this can finally go in :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reapproving
In the seedFinder we check the compatibility between the (r,z) slope of the two seed segments with a scattering term calculated assuming the minimum pT particle expected to be reconstructed and a second check using a scattering term scaled by the actual measured pT. As discussed here in PR #1213, there was an error in these cuts. This PR fixes that same error in SeedFinderOrthogonal.
This was included in the default seeding in PR #1213
This PR splits
enableCutsForSortedSP
into two booleans:Instead of using
enableCutsForSortedSP
to enable both the cotTheta sorting of SPs and the cut based on the sorting, this PR makesenableCutsForSortedSP
enable only the sorting andskipPreviousTopSP
is added for the additional cut to skip top SPs if cotThetaBB < cotThetaTFor ITk strips the cuts are not applied.
Edit: The original error calculation was fixes,
enableCutsForSortedSP
was removed since it is not needed andskipPreviousTopSP
was added to skip top SPs