feat(simg4ox): support multithreaded Geant4 runs - #441
Conversation
Select serial or MT Geant4 run managers through --threads and construct worker-local user actions through G4VUserActionInitialization. Preserve per-event source photons for ordered GPU launches, serialize access to the process-wide Opticks context, and merge CPU/GPU hit arrays by event ID through a custom G4Run. Add a two-worker CTest fixture, Geant macro, runner, and serial/MT usage documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28e06447fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds multithreaded Geant4 execution support to simg4ox while keeping Opticks GPU transport serialized, and updates tests/docs to reflect the new execution model.
Changes:
- Add
--threads Nand select serial vs MT Geant4 run managers viaG4RunManagerFactory. - Refactor Geant4 user actions to be worker-local via
G4VUserActionInitialization, with serialized GPU processing in event-ID order and run-level hit merging. - Add an MT integration test plus expanded documentation (including a Typst timeline) describing the runtime model.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_simg4ox_multithread.sh | New integration test driver script for MT runs and output validation. |
| tests/run_mt.mac | New 5-event macro tailored for MT execution (init + beamOn). |
| tests/CMakeLists.txt | Registers the new MT integration test and sets a dedicated working directory. |
| src/simg4ox.cpp | Adds --threads CLI and uses Geant4 run-manager factory to select serial vs MT. |
| src/g4app.h | Refactors actions for MT: worker-local actions, serialized GPU section, run merge by event ID. |
| src/CMakeLists.txt | Updates simg4ox target description to reflect serial/MT CPU + serialized GPU launches. |
| README.md | Documents --threads and the serialized GPU constraint at a high level. |
| examples/README.md | Updates simg4ox example docs to cover Geant4 MT and ordered GPU serialization. |
| docs/performance-and-debugging.md | Adds guidance for interpreting MT timings given serialized GPU work. |
| docs/inputs-outputs.md | Documents simg4ox execution model and run-level output arrays in serial/MT. |
| docs/getting-started.md | Adds how to run the new MT integration test and notes MT build requirements. |
| docs/assets/simg4ox-event-processing.typ | Adds Typst source for an MT vs serial execution timeline graphic. |
Suppressed comments (1)
src/g4app.h:401
- This
assert(primary_info && ...)guards a dereference, butassertis compiled out in release builds (whenNDEBUGis set). In that caseprimary_infocan be null and the next line will be undefined behavior. Prefer an explicit runtime check that either fails the event/run or returns safely.
const auto* primary_info = dynamic_cast<const PrimaryPhotonInfo*>(event->GetUserInformation());
assert(primary_info && "MT events must retain their generated photons for GPU processing");
SEvt::SetInputPhoton(MakePhotonArray(primary_info->photons));
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| #include <condition_variable> | ||
| #include <cstring> | ||
| #include <filesystem> |
380e12c to
d2fc62e
Compare
| const PrimaryPhotonInfo* primary_info = nullptr; | ||
| if (order_gpu_events) | ||
| { | ||
| primary_info = dynamic_cast<const PrimaryPhotonInfo*>(event->GetUserInformation()); | ||
| if (!primary_info) | ||
| throw std::runtime_error("MT event is missing its generated photons for GPU processing"); | ||
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests/test_simg4ox_multithread.sh:34
- The reused checker only counts matching log lines, sums hit counts, and checks aggregate array shape/dtype; it never reads event IDs or verifies event grouping/content. Consequently this MT test still passes if GPU launches occur out of event-ID order or if the worker merge produces the wrong event order, which are the central new guarantees. Add event-identifying test data/instrumentation and assert launch and aggregate ordering explicitly.
"${PYTHON}" "${REPO_DIR}/tests/check_simg4ox_multievent.py" \
--log "${RUN_LOG}" \
--output-dir "${PWD}" \
--events 5
d263d26 to
96dc7db
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests/test_simg4ox_multithread.sh:29
- This integration check only validates hit counts and array shapes; it does not verify the new event-to-payload association or event-ID ordering.
generate_photonsis called with its default seed of 0 for every event, so all event inputs are identical and a stale/swapped payload or incorrectly ordered merge would still pass. Make the test inputs observably event-specific and assert that the GPU and saved run arrays preserve event-ID order.
"${PYTHON}" "${REPO_DIR}/tests/check_simg4ox_multievent.py" \
--log "${RUN_LOG}" \
--output-dir "${PWD}" \
--events 5
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96dc7dbb3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| G4RunManager* run_mgr = multithreaded | ||
| ? G4RunManagerFactory::CreateRunManager(G4RunManagerType::MTOnly, num_threads) | ||
| : G4RunManagerFactory::CreateRunManager(G4RunManagerType::SerialOnly); |
There was a problem hiding this comment.
When --threads N is greater than one, the second parameter of G4RunManagerFactory::CreateRunManager is fail_if_unavail, not the thread count, so num_threads is merely converted to true while Geant4 selects its default worker count. The subsequent equality check therefore exits for common requests such as --threads 2 whenever the host default differs; pass the count as the third argument or call SetNumberOfThreads on the MT manager.
Useful? React with 👍 / 👎.
| const G4int configured_threads = run_mgr->GetNumberOfThreads(); | ||
| if (configured_threads != num_threads) |
There was a problem hiding this comment.
For the default --threads 1 path, a serial G4RunManager reports zero worker threads from GetNumberOfThreads(), since the sole execution thread is not an MT worker. Comparing that value with one makes every default serial invocation—including the existing integration tests and documented commands—exit before initialization; restrict this validation to MT mode or normalize the serial result to one.
Useful? React with 👍 / 👎.
Summary
--threads Nto select serial or multithreaded Geant4 execution while preserving the serial default.G4VUserActionInitialization.g_hits.npyands_hits.npyrun outputs.Runtime model
Geant4 event processing runs concurrently when
--threads Nis greater than one. Opticks GPU photon transport remains serialized because its event context is process-wide.MT workers retain their generated photons on the
G4Eventuntil entering the ordered GPU section. Full CPU-side Opticks photon-history recording remains available in serial mode, while both modes collect sensitive-detector hits and produce run-wide hit arrays.