Skip to content

fix: surface Geant4 init failures and repair production-cut regions#83

Open
olantwin wants to merge 4 commits into
mainfrom
fix/region-patterns-init-errors
Open

fix: surface Geant4 init failures and repair production-cut regions#83
olantwin wants to merge 4 commits into
mainfrom
fix/region-patterns-init-errors

Conversation

@olantwin

Copy link
Copy Markdown
Contributor

Summary

fixed_target_mt.jsonnet (and every workflow using geant4_crossing) died ~30 s in with Geant4's fatal Run0031: G4RunManager constructed twice abort — with the actual error hidden. Root cause chain:

  1. The production-cut patterns Target/HadronAbsorber date from v0.1.0 and match no logical volume in the GeoModel geometry (/SHiP/target/..., /SHiP/muon_shield/magn_absorb). Before a0116a0 they silently produced empty regions (no cuts applied); since a0116a0 they throw during rm->Initialize().
  2. The throw left init_flag_ unset, so the next in-flight simulate() call re-ran init_master, constructed a second G4MTRunManager next to the leaked first one, and died on the fatal G4 abort — masking the configuration error entirely.
  3. Even without the retry, a failed init left the geometry stores populated with initialized_ unset, so their static destructors segfaulted at process exit (the Segmentation violation at teardown when running with GeoModel geometry #68 pattern).

Changes

  • workflows/docs: point geant4_crossing regions at /SHiP/target and /SHiP/muon_shield/magn_absorb (the magnetised hadron absorber).
  • geant4_module: a failed master init is now sticky — the exception is logged and rethrown on every simulate() call, so phlex reports the real error and exits instead of aborting.
  • geant4_module: the destructor cleans the geometry stores whenever the run manager was constructed, not only after a fully successful init, so a config-error exit is clean instead of a shutdown segfault.
  • geant4_module: log a progress line (event count + average rate) every progress_interval events (default 100, 0 disables) — a healthy multi-hour run was previously indistinguishable from a stall.

Verification

  • fixed_target_mt.jsonnet: no more Run0031; init completes, 4 workers simulate at ~20 events/s with progress lines.
  • fixed_target_st.jsonnet: runs to completion, exit 0, output file written, clean shutdown.
  • Deliberately bad region pattern: exits 1 with Production-cut region 'DoesNotExist' matches no logical volumes in the log; no abort, no segfault.

Note: touches geant4_module.cpp near the physics-list validation that fix/g4-fail-cleanly also modifies; whichever merges second may need a trivial rebase.

olantwin added 4 commits July 23, 2026 17:04
The geant4_crossing patterns Target and HadronAbsorber date from the
v0.1.0 geometry and match no logical volume in the GeoModel geometry,
whose volumes are named /SHiP/target/... and
/SHiP/muon_shield/magn_absorb. Since the no-match check became fatal,
every workflow using geant4_crossing dies during master initialisation;
before that, the regions were silently empty and no production cuts
were applied. Point the patterns at the current target complex and
magnetised hadron absorber.
A throw from init_master left init_flag_ unset, so the next simulate
call — phlex keeps feeding in-flight events after one throws — re-ran
the initialisation. The first G4MTRunManager is a leaked singleton, so
the retry died on Geant4's fatal Run0031 "G4RunManager constructed
twice" abort, hiding the original error entirely (e.g. the
production-cut region config error was never printed). Capture the
exception in the call_once body, log it, and rethrow it on every
simulate call: phlex drains in-flight events, reports the real error,
and exits instead of aborting.
A production run prints nothing between master initialisation and the
end of the job, so a healthy multi-hour simulation is indistinguishable
from a stall. Log the completed-event count and average rate every
progress_interval events (default 100, 0 disables).
The destructor cleaned the geometry stores on the geometry thread only
after a fully successful master init. A failed init typically fails
after detector construction has registered the volumes, so the stores
stayed populated and their static destructors segfaulted at process
exit on the main thread (the #68 pattern), turning a clean
configuration-error exit into a crash. Key the cleanup on the run
manager's existence instead: from that point the geometry thread owns
the thread-local split-class data and the stores may hold volumes.
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@olantwin, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 17 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e48b5b8a-6fd5-4a58-b0a8-e64dd8ada932

📥 Commits

Reviewing files that changed from the base of the PR and between 79ac2f5 and 23279f7.

📒 Files selected for processing (3)
  • docs/geant4_integration.md
  • src/geant4_module.cpp
  • workflows/lib.libsonnet
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/region-patterns-init-errors

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

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

@github-actions

Copy link
Copy Markdown

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: 964 → 927 (Δ=3.838%)
  h_hit_z.integral: 964 → 927 (Δ=3.838%)
  h_hit_detector.integral: 964 → 927 (Δ=3.838%)
  h_hit_momentum.integral: 964 → 927 (Δ=3.838%)


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>

@olantwin

Copy link
Copy Markdown
Contributor Author

@THanae This might fix the issues you are seeing.

@THanae

THanae commented Jul 23, 2026

Copy link
Copy Markdown

@THanae This might fix the issues you are seeing.

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants