fix(geant4): fail cleanly on unknown physics lists, count worker kernels

[olantwin]

• Query IsReferencePhysList before GetReferencePhysList, which aborts the process via a fatal G4Exception on unknown names. A bad config now turns into a catchable std::runtime_error, which the sticky-init path on main reports cleanly instead of a core dump.
• Log the worker-kernel count at shutdown and warn when TBB thread churn builds more kernels than configured slots: each new pool thread that ever runs simulate builds its own kernel, silently costing memory and a fresh RNG stream. Geant4 11 tolerates thread ids beyond SetNumberOfThreads in this direct-injection flow (verified empirically), but it should be visible.
• Correct the reproducibility mechanism note in docs/geant4-perf-notes.md: the dominant effect is event-order nondeterminism against a continuous per-kernel RNG stream, observed even in runs where only a single kernel was built. Per-event seeding from the stable cell index (feat(geant4): seed the RNG per event from the data-cell index #54) is the fix, not the arrival-order G4Event id.

Rebased onto current main; the sticky-init and geometry-store-cleanup fixes it now carries make the earlier "clean retry" framing obsolete, so this change keeps only the up-front validation (which complements sticky init), the kernel accounting, and the docs correction.

Summary by CodeRabbit

• Bug Fixes

  • Unknown physics-list names now fail cleanly with a clear, catchable error instead of terminating.
  • Added safeguards and warnings when worker kernels are created beyond the configured concurrency.
  • Shutdown reporting now includes how many per-thread worker kernels initialized successfully.

• Tests

  • Added a new CTest and bash script to verify unknown physics-list failures under concurrent initialization (including "no abort" checks).

• Documentation

  • Updated reproducibility notes to explain variability from event ordering and continuous RNG stream differences.
  • Documented kernel-count logging and per-event cell-index seeding as the reproducibility fix.

[coderabbitai]

📝 Walkthrough

Walkthrough

Geant4Sim now validates unknown physics lists before run-manager construction, reports completed and excess worker kernels, adds a concurrent clean-failure test, and updates reproducibility notes on event ordering, RNG streams, and TBB thread churn.

Changes

Geant4 runtime observability

Layer / File(s)	Summary
Physics-list validation and failure coverage src/geant4_module.cpp, scripts/physics_list_failure.sh, CMakeLists.txt	Master initialization rejects unknown reference physics lists with a catchable runtime error. A multi-threaded test verifies the expected failure without an abort or run-manager reconstruction error.
Worker-kernel lifecycle reporting src/geant4_module.cpp	Worker initialization warns when TBB creates kernels beyond configured concurrency, counts only completed initializations, and logs the completed-kernel count during destruction.
Reproducibility documentation docs/geant4-perf-notes.md	Performance notes explain event-order and continuous-RNG variation, kernel-count logging, per-event seeding, and thread-churn warnings.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• ShipSoft/aegir#36: Modifies the failure handling around Geant4 master initialization.
• ShipSoft/aegir#48: Addresses concurrency and worker-kernel churn in src/geant4_module.cpp.
• ShipSoft/aegir#51: Updates the related reproducibility and worker-kernel churn documentation.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The description is detailed, but it omits the required Checklist section from the repository template.	Add the Checklist section with the three required items: conventional commits, pre-commit lint, and CI checks.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately summarizes the two main changes: physics-list validation and worker-kernel accounting.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/g4-fail-cleanly

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Physics Metrics Comparison

1 configuration(s) have differences

• Configurations compared: 1
• With differences: 1

Configurations with Differences

comparison_physics-smoke

• Files compared: 1
• Histograms compared: 8
• Fits compared: 0
• Total differences: 4

Show differences

gun_st_nofield_smoke_validation.root:
  h_hit_edep.integral: 911 → 914 (Δ=0.3293%)
  h_hit_z.integral: 911 → 914 (Δ=0.3293%)
  h_hit_detector.integral: 911 → 914 (Δ=0.3293%)
  h_hit_momentum.integral: 911 → 914 (Δ=0.3293%)

---

About this comparison

Physics metrics are extracted from ROOT files and compared.
Default comparison uses 5% tolerance for histogram statistics
and 3σ for fit parameters.

Metrics stored in git notes: refs/notes/ci/physics-metrics/<config>

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@src/geant4_module.cpp`:
- Around line 122-124: Update the shutdown summary near the `next_thread_id_`
check to use a dedicated atomic counter for successfully initialized kernels,
rather than allocated worker IDs. Increment that counter only after
`init_worker()` completes successfully, and report its value while preserving
the existing logging condition and message.
- Around line 284-290: Move the physics-list validation using
G4PhysListFactory::IsReferencePhysList before init_master() allocates the leaked
G4MTRunManager or detector_. Preserve the existing unknown-list exception and
only construct/register those resources after validation succeeds; add a
regression test covering a failed initialization followed by retry.

🪄 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 Plus

Run ID: ec7f0450-562d-4f8e-b5f8-29981c9f9a63

📥 Commits

Reviewing files that changed from the base of the PR and between 79ac2f5 and 2f660ec.

📒 Files selected for processing (2)

• docs/geant4-perf-notes.md
• src/geant4_module.cpp

[olantwin]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🤖 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 67-69: Update the per-event RNG follow-up guidance near the
discussion of next_event_id_ so it requires seeding from a stable upstream
input-event identifier, or explicitly preserving input ordering before assigning
event IDs. Do not present arrival-order IDs from next_event_id_.fetch_add(1) as
a reproducibility fix.

In `@scripts/physics_list_failure.sh`:
- Around line 51-52: Make the concurrency test around the phlex invocation
deterministic by adding an observable retry counter or equivalent test hook for
the std::call_once retry path. Update the assertion in
scripts/physics_list_failure.sh to require that the counter records at least one
retry, while preserving the existing command and return-code checks.

🪄 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 Plus

Run ID: 344318fb-3d94-46d6-8844-05143bdf2210

📥 Commits

Reviewing files that changed from the base of the PR and between 2f660ec and 493213c.

📒 Files selected for processing (4)

• CMakeLists.txt
• docs/geant4-perf-notes.md
• scripts/physics_list_failure.sh
• src/geant4_module.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• src/geant4_module.cpp