-
Notifications
You must be signed in to change notification settings - Fork 174
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: configurable maximum number of bins in SP grid #2325
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2325 +/- ##
==========================================
- Coverage 49.64% 49.63% -0.01%
==========================================
Files 453 453
Lines 25538 25540 +2
Branches 11708 11709 +1
==========================================
Hits 12678 12678
- Misses 4578 4580 +2
Partials 8282 8282
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
📊: Physics performance monitoring for 4a95ff9physmon summary
|
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.
Looks good overall, only spotted one inconsistency with the (now changed) default of 300. I don't have a strong opinion on the default. I guess infinite is a bit dangerous since you could potentially try to allocate infinite memory that way, although I suppose it's bounded by the event activity.
I'd probably pick a "large but not infinite" number, like mayb 10k or so and then mention that in the comment.
Up to you @LuisFelipeCoelho.
Co-authored-by: Paul Gessinger <hello@paulgessinger.com>
Somehow the clang-tidy job seems to get stuck repeatedly, in that it's result isn't posted back here. It's only that job it seems though, which is weird. |
Failure is unrelated to us. |
Protection for large number of bins which is set by default to
std::numeric_limits<int>::max()
This comes from PR #2166, I am splitting the changes into smaller PRs.
@CarloVarni