Repository navigation
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The workflow changes use --numberOfThreads while the PR title/description state the goal is to exercise npsim -j, so the implementation and stated intent should be aligned.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR updates the CI simulation runs to request multi-threaded execution in npsim, aiming to exercise thread-parallel behavior in a workflow where output diffs are visible.
Changes:
- Add a per-run thread count argument to
npsimin the “gun” workflow simulation step. - Add the same thread count argument to
npsimin the DIS workflow simulation step.
| File | Description |
|---|---|
| .github/workflows/build-push.yml | Adds CPU-thread-based parallelism configuration to npsim runs in CI workflows. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| run: | | ||
| source /opt/detector/epic-main/bin/thisepic.sh | ||
| npsim \ | ||
| --numberOfThreads $(getconf _NPROCESSORS_ONLN) \ |
There was a problem hiding this comment.
Is that event-by-event reproducible?
There was a problem hiding this comment.
It is designed to be (that's the part that took the two years; just getting stuff to be multithreaded isn't hard).
There was a problem hiding this comment.
It is also enforced in DD4hep CI by running test simulations at multiple thread counts and only allowing out-of-order events, but otherwise requiring identity.
npsim -j in workflowsnpsim -j/--numberOfThreads in workflows

Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.
This PR adds
-jto the npsim runs in the workflows. Slightly more efficient but more importantly it's functionality we want to ensure is exercised in a place where we can see diffs.What is the urgency of this PR?
What kind of change does this PR introduce?
npsim -j)Please check if any of the following apply