perf: Reduce number of redundant calls in strip seeding#5239
Merged
paulgessinger merged 9 commits intoacts-project:mainfrom Mar 17, 2026
Merged
perf: Reduce number of redundant calls in strip seeding#5239paulgessinger merged 9 commits intoacts-project:mainfrom
paulgessinger merged 9 commits intoacts-project:mainfrom
Conversation
andiwand
changed the title
Contributor
andiwand
reviewed
Mar 14, 2026
Contributor
andiwand
left a comment
andiwand
left a comment
There was a problem hiding this comment.
This is great! Thank you for adding this @mvessell96 !!
I only have a two minor comments.
As an alternative to the POD + free functions approach it might be possible to have a small helper class that holds some state for our use case: check middle -> bottom -> top. And the SP filling functions and checks become member functions. But I don't think we really benefit from that in any way
andiwand
previously approved these changes
Mar 14, 2026
Contributor
|
/ci-bridge-run |
Contributor
|
ah sorry - @mvessell96 there are some formatting errors https://github.com/acts-project/acts/actions/runs/23097825688/job/67093276148?pr=5239 can you have a look? |
Contributor
|
/ci-bridge-run |
andiwand
approved these changes
Mar 16, 2026
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reduce number of redundant calls in strip seeding
--- END COMMIT MESSAGE ---
Profiling indicates that strip seeding spends an unreasonably long amount of time inside of the check for compatibility with the sensor geometry, which is individually called for each bottom/middle/top sp in the triple triplet formation loop.
We can make this a bit better behaved by noting strip geometry vectors for the middle and bottoms sps are invariant across the top-doublet loop and cleverly load the vectors once and precompute some cross products w/ the scalar triple product identity, which eliminates two cross products per iteration for each bottom/middle sp.
Since the top sps change every iteration, precomputing the cross products could be a waste because the first check might reject the candidate immediately, so instead we can overload the coordinate check to only compute the additional cross products if needed.
Lastly, we can eliminate unnecessary sqrt calls by noting that the middle check only cares about the ratios s1/bd1 and s0/bd1, sqrt (1+A0^2) is just a multiplicative factor, so can compute it only if the ratios are within tolerance and the check passes.
This appears to reduce the total CPU time taken in the ATLAS LRT strip seeding by ~40% in single-threaded (on 5 events of LLP signal sample -- so take with a grain of salt) and should theoretically be numerically identical