-
Notifications
You must be signed in to change notification settings - Fork 176
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: Early exit in the seeding if there are no space points for top/bottom… #3284
fix: Early exit in the seeding if there are no space points for top/bottom… #3284
Conversation
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.
LGTM - Maybe @LuisFelipeCoelho whats to have another look
📊: Physics performance monitoring for 8577c19physmon summary
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3284 +/- ##
==========================================
- Coverage 47.37% 47.36% -0.01%
==========================================
Files 511 511
Lines 30254 30260 +6
Branches 14679 14680 +1
==========================================
Hits 14333 14333
- Misses 5383 5389 +6
Partials 10538 10538 ☔ View full report in Codecov by Sentry. |
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 for fixing this @CarloVarni. It looks good to me. Do you know if this improves the CPU performance? Maybe for high-pileup it doesn't change much?
I suspect a very minimal improvement, and only for single particle cases. |
Just flickered the |
…ottom… (acts-project#3284) The following cut in the code was supposed to allow for an early exit of the code in case there are no space points that can be used as bottom or top candidates ```cpp if (state.bottomNeighbours.size() == 0 || state.topNeighbours.size() == 0) ``` Too bad this was buggy and it was always false, since those arrays were always filled even if there were no space points. This PR fixes it, since now the arrays `state.topNeighbours` and `state.bottomNeighbours` are filled only if the bin contains entries.
…ottom… (acts-project#3284) The following cut in the code was supposed to allow for an early exit of the code in case there are no space points that can be used as bottom or top candidates ```cpp if (state.bottomNeighbours.size() == 0 || state.topNeighbours.size() == 0) ``` Too bad this was buggy and it was always false, since those arrays were always filled even if there were no space points. This PR fixes it, since now the arrays `state.topNeighbours` and `state.bottomNeighbours` are filled only if the bin contains entries.
…ottom… (acts-project#3284) The following cut in the code was supposed to allow for an early exit of the code in case there are no space points that can be used as bottom or top candidates ```cpp if (state.bottomNeighbours.size() == 0 || state.topNeighbours.size() == 0) ``` Too bad this was buggy and it was always false, since those arrays were always filled even if there were no space points. This PR fixes it, since now the arrays `state.topNeighbours` and `state.bottomNeighbours` are filled only if the bin contains entries.
The following cut in the code was supposed to allow for an early exit of the code in case there are no space points that can be used as bottom or top candidates
Too bad this was buggy and it was always false, since those arrays were always filled even if there were no space points. This PR fixes it, since now the arrays
state.topNeighbours
andstate.bottomNeighbours
are filled only if the bin contains entries.