From fd66f7a93cb8227bf243c531e5d70878cd33102b Mon Sep 17 00:00:00 2001 From: Oliver Lantwin Date: Thu, 23 Jul 2026 17:04:40 +0200 Subject: [PATCH 1/4] fix(workflows): match production-cut regions to GeoModel volume names 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. --- docs/geant4_integration.md | 2 +- workflows/lib.libsonnet | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/geant4_integration.md b/docs/geant4_integration.md index 8ef7f25..e37f839 100644 --- a/docs/geant4_integration.md +++ b/docs/geant4_integration.md @@ -133,7 +133,7 @@ Example workflow configuration: ke_threshold: 0.5, energy_cut: true, particle_ke_cut: 1.0, - regions: { Target: 50, HadronAbsorber: 50 }, + regions: { '/SHiP/target': 50, '/SHiP/muon_shield/magn_absorb': 50 }, }, }, } diff --git a/workflows/lib.libsonnet b/workflows/lib.libsonnet index 4b050ab..c27cfe4 100644 --- a/workflows/lib.libsonnet +++ b/workflows/lib.libsonnet @@ -98,7 +98,7 @@ ke_threshold: 0.5, energy_cut: true, particle_ke_cut: 1.0, - regions: { Target: 50, HadronAbsorber: 50 }, + regions: { '/SHiP/target': 50, '/SHiP/muon_shield/magn_absorb': 50 }, }, noop_output:: { cpp: 'sim_output_module', mode: 'noop' }, From 1ba6c9cb7d2dcd8bdf89180322cb64593d3a29c3 Mon Sep 17 00:00:00 2001 From: Oliver Lantwin Date: Thu, 23 Jul 2026 17:07:26 +0200 Subject: [PATCH 2/4] fix(geant4): make a failed master init sticky instead of retrying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/geant4_module.cpp | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/geant4_module.cpp b/src/geant4_module.cpp index 6e0ce57..1c5c121 100644 --- a/src/geant4_module.cpp +++ b/src/geant4_module.cpp @@ -47,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -147,8 +148,23 @@ class Geant4Sim { std::vector const& particles) { AEGIR_TRACE_EVENT("g4", "simulate"); - std::call_once(init_flag_, - [this, &geo, &field]() { init_master(geo, field); }); + // A failed master init is sticky: retrying would construct a second + // G4MTRunManager next to the leaked first one and die on Geant4's fatal + // "constructed twice" abort, masking the original error. Store the + // exception instead and rethrow it on this and every later call — phlex + // logs it at shutdown and terminates the job cleanly. + std::call_once(init_flag_, [this, &geo, &field]() { + try { + init_master(geo, field); + } catch (std::exception const& e) { + spdlog::error("geant4_module: master initialisation failed: {}", + e.what()); + init_error_ = std::current_exception(); + } catch (...) { + init_error_ = std::current_exception(); + } + }); + if (init_error_) std::rethrow_exception(init_error_); if (!tl_kernel) init_worker(); @@ -243,10 +259,8 @@ class Geant4Sim { // pre-check the cases we can to fail with a catchable, clear error // instead. Only the existing-file and missing-parent-directory cases are // validated here — other fatal write errors (e.g. a read-only directory) - // remain Geant4's responsibility. Doing this before the G4MTRunManager is - // constructed keeps the std::call_once retry clean: the run manager is a - // leaked singleton, so a throw after it exists would make the retry abort - // with "run manager already exists". + // remain Geant4's responsibility. Failing before the G4MTRunManager is + // constructed keeps the failure cheap: nothing has been built yet. if (!cfg_.export_gdml.empty()) { if (std::filesystem::exists(cfg_.export_gdml)) throw std::runtime_error( @@ -268,8 +282,8 @@ class Geant4Sim { // slot count is shared while its data array is thread-local), and other // plugins — e.g. aegir-genie's GENIE geometry analyzer — create theirs // on this thread too (aegir-genie issue #11, Geant4 bug #2747). The call - // blocks until initialisation is done; exceptions propagate, so a failed - // init can be retried (init_flag_ stays unset). + // blocks until initialisation is done; exceptions propagate to the + // caller in simulate(), which records them as a sticky init failure. ship::geometry_thread().run([this] { AEGIR_TRACE_THREAD_NAME("g4_master"); AEGIR_TRACE_EVENT("g4", "init_master"); @@ -347,6 +361,9 @@ class Geant4Sim { Geant4SimConfig cfg_; std::once_flag init_flag_; + // Written at most once inside the call_once body, read only after the + // call_once returns — no further synchronisation needed. + std::exception_ptr init_error_; G4VPhysicalVolume* world_pv_ = nullptr; G4VUserPhysicsList* physics_list_ = nullptr; ConfigurableDetectorConstruction* detector_ = nullptr; // owned by G4 From 01177640ad4a29851fbd08125d138e1437585a90 Mon Sep 17 00:00:00 2001 From: Oliver Lantwin Date: Thu, 23 Jul 2026 17:09:54 +0200 Subject: [PATCH 3/4] feat(geant4): log simulation progress every N events 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). --- docs/geant4_integration.md | 1 + src/geant4_module.cpp | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/docs/geant4_integration.md b/docs/geant4_integration.md index e37f839..b1b2109 100644 --- a/docs/geant4_integration.md +++ b/docs/geant4_integration.md @@ -109,6 +109,7 @@ unloading, causing crashes. This is a known Geant4 limitation. | `particle_ke_cut` | double | `0.0` | KE below which secondary particles are not recorded (GeV) | | `regions` | map | `{}` | Volume name pattern to production cut (mm) mapping | | `export_gdml` | string | *(unset)* | Write the constructed geometry to this GDML file after initialisation. Errors if the file exists. Lets external tools (e.g. the GENIE event generator) use exactly the geometry Geant4 tracks in | +| `progress_interval` | int | `100` | Log a progress line (event count and average rate) every this many simulated events; `0` disables | For consumers that read the exported file with ROOT's TGeo importer, run `scripts/gdml_fix_element_names.py` on it first: ROOT confuses materials diff --git a/src/geant4_module.cpp b/src/geant4_module.cpp index 1c5c121..46c8c4b 100644 --- a/src/geant4_module.cpp +++ b/src/geant4_module.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -112,6 +113,8 @@ struct Geant4SimConfig { // When set, write the constructed geometry to this GDML file after // initialisation (e.g. to feed the same geometry to external tools). std::string export_gdml; + // Log a progress line every this many simulated events (0 disables). + int progress_interval = 100; }; class Geant4Sim { @@ -248,6 +251,17 @@ class Geant4Sim { result.hits = std::move(tl_hits); result.particles = std::move(tl_particles); } + + auto const done = completed_.fetch_add(1) + 1; + if (cfg_.progress_interval > 0 && + done % static_cast(cfg_.progress_interval) == 0) { + auto const elapsed = std::chrono::duration( + std::chrono::steady_clock::now() - sim_start_) + .count(); + spdlog::info("geant4_module: {} events simulated ({:.1f} events/s)", done, + elapsed > 0 ? done / elapsed : 0.0); + } + return result; } @@ -327,6 +341,7 @@ class Geant4Sim { // The run manager stays alive (and leaked) for the whole process. }); + sim_start_ = std::chrono::steady_clock::now(); initialized_ = true; spdlog::info("Geant4 direct simulation ready ({} worker slots)", @@ -370,6 +385,12 @@ class Geant4Sim { std::shared_ptr field_; // outlives G4 run std::atomic next_thread_id_{0}; std::atomic next_event_id_{0}; + // Completed-event count (completion order, not event id, so the logged + // count is monotonic) and the reference point for the average event rate. + // sim_start_ is written in init_master and published by the call_once + // every simulate call passes through first. + std::atomic completed_{0}; + std::chrono::steady_clock::time_point sim_start_; // Set only after a successful init_master, so the destructor cleans the // stores exactly when there is something to clean. bool initialized_ = false; @@ -413,6 +434,7 @@ PHLEX_REGISTER_ALGORITHMS(m, config) { aegir::get_quantity(config, "particle_ke_cut", 0.0 * su::GeV), .regions = std::move(regions), .export_gdml = config.get("export_gdml", std::string{}), + .progress_interval = config.get("progress_interval", 100), }; if (cfg.concurrency > active_parallelism) From 23279f775cf7821c43125b42ef743f075ffd9636 Mon Sep 17 00:00:00 2001 From: Oliver Lantwin Date: Thu, 23 Jul 2026 17:40:10 +0200 Subject: [PATCH 4/4] fix(geant4): clean geometry stores whenever the run manager exists 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. --- src/geant4_module.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/geant4_module.cpp b/src/geant4_module.cpp index 46c8c4b..8a1a0f4 100644 --- a/src/geant4_module.cpp +++ b/src/geant4_module.cpp @@ -122,7 +122,7 @@ class Geant4Sim { explicit Geant4Sim(Geant4SimConfig cfg) : cfg_{std::move(cfg)} {} ~Geant4Sim() { - if (!initialized_) return; + if (!master_constructed_) return; // Empty the geometry stores now, on the geometry thread. Their static // singletons' destructors run at process exit on the program's main // thread, which never initialised Geant4's thread-local split-class @@ -302,6 +302,7 @@ class Geant4Sim { AEGIR_TRACE_THREAD_NAME("g4_master"); AEGIR_TRACE_EVENT("g4", "init_master"); auto* rm = new G4MTRunManager(); + master_constructed_ = true; rm->SetNumberOfThreads(cfg_.concurrency); rm->SetUserInitialization(detector_); @@ -342,7 +343,6 @@ class Geant4Sim { }); sim_start_ = std::chrono::steady_clock::now(); - initialized_ = true; spdlog::info("Geant4 direct simulation ready ({} worker slots)", cfg_.concurrency); @@ -391,9 +391,14 @@ class Geant4Sim { // every simulate call passes through first. std::atomic completed_{0}; std::chrono::steady_clock::time_point sim_start_; - // Set only after a successful init_master, so the destructor cleans the - // stores exactly when there is something to clean. - bool initialized_ = false; + // Set on the geometry thread as soon as the G4MTRunManager exists. From + // that point the geometry stores may hold volumes — even when a later init + // step throws, detector construction has typically already registered them + // — so the destructor must clean the stores on the geometry thread (#68) + // for failed and successful inits alike. Guarding on full init success + // instead would leave the stores populated after a failed init and their + // static destructors would segfault at process exit. + bool master_constructed_ = false; }; } // namespace