fix(bench): pin single-threaded recipes to -j 1#51
Conversation
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR documents nondeterminism caused by multithreaded worker-kernel and RNG-stream assignment, and adds ChangesReproducible benchmark execution
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
6df4050 to
3d85317
Compare
3d85317 to
27ff670
Compare
0cb35be to
4c812ed
Compare
27ff670 to
4c2fb47
Compare
4c812ed to
d41b3d2
Compare
4c2fb47 to
48e43fc
Compare
d41b3d2 to
22e4daa
Compare
48e43fc to
19ae236
Compare
Physics Metrics Comparison1 configuration(s) have differences
Configurations with Differences
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/geant4-perf-notes.md`:
- Around line 59-60: Update the phlex -j 1 reproducibility statement in the
documentation to refer specifically to the checked validation histograms
h_mc_multiplicity, h_mc_momentum, and h_mc_pdg being reproducible, rather than
claiming the full output is bitwise identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2af2b9ce-440d-47a1-9367-56d342e3ab7a
📒 Files selected for processing (2)
docs/geant4-perf-notes.mdjustfile
The geant4 module now derives its worker count from the framework parallelism, so the gun_st recipes must say -j 1 explicitly to stay single-threaded. Only -j 1 runs are bitwise reproducible: at higher -j, TBB hands successive events to different pool threads, each with its own lazily-built worker kernel and RNG stream, even when the module's concurrency is 1 (recorded in docs/geant4-perf-notes.md). This covers the profile-flamegraph and profile-trace recipes too: they default to the gun_st_bench workflow, so they need -j 1 to profile the single-threaded path rather than a multi-threaded run.
19ae236 to
9feddd2
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
The geant4 module now derives its worker count from the framework parallelism, so the gun_st recipes must say
-j 1explicitly to stay single-threaded.Only
-j 1runs are bitwise reproducible: at higher-j, events reach the worker kernel(s) in nondeterministic order and consume a continuous per-kernel RNG stream, even when the module's concurrency is 1 (measured and recorded inresults/NOTES.md). Per-event seeding is the proper fix and follows in a separate PR.Part 6 of the series (base: perf/rntuple-bind).
Summary by CodeRabbit
Documentation
Chores