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

CMakeLists.txt

@@ -203,3 +203,10 @@ add_test(
     NAME file_source_roundtrip
     COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/scripts/file_source_roundtrip.sh
 )
+
+# Physics-list failure: an unknown physics_list must fail cleanly (catchable
+# error, no abort) even when concurrent init retries race on std::call_once.
+add_test(
+    NAME physics_list_failure
+    COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/scripts/physics_list_failure.sh
+)

docs/geant4-perf-notes.md

@@ -59,14 +59,22 @@ Measured with `gun_st_full` (200 events, fixed gun seed):
 - `phlex -j 1`: **reproducible** — the validation histograms
   `h_mc_multiplicity`, `h_mc_momentum`, and `h_mc_pdg` are bin-by-bin
   identical across runs (checked by `scripts/compare_histograms.py`).
-- `concurrency: 1` with `-j 12`: **not reproducible.** Even though only
-  one `simulate` call runs at a time, TBB hands successive calls to
-  different pool threads; each new thread lazily builds its own
-  `G4WorkerRunManagerKernel` with its own RNG stream, so the event
-  sequence samples different streams run to run. This directly confirms
-  the worker-kernel churn concern: the kernel count is bounded by the
-  number of distinct TBB threads that ever touch `simulate`, not by the
-  module's `concurrency`.
+- `concurrency: 1` with `-j 12`: **not reproducible**, even in runs
+  where only a single worker kernel was built (the module now logs the
+  kernel count at shutdown). The dominant mechanism is event order: with
+  12 events in flight upstream, the order in which events reach the
+  single `simulate` slot varies run to run, and each event starts at a
+  different point in the kernel's continuous RNG stream. Per-event
+  seeding fixes this: PR #54 (`feat/g4-per-event-seeding`) reseeds the
+  CLHEP engine from the stable phlex `data_cell_index` (its `hash()`, the
+  same per-event index the generators already seed from), so an event's
+  random stream no longer depends on which `simulate` slot it reaches.
+  Seeding from the arrival-order `G4Event` id
+  (`next_event_id_.fetch_add(1)`) would not, because that id itself varies
+  with arrival order. Kernel multiplication through TBB thread churn
+  (each distinct pool thread that ever runs `simulate` builds its own
+  kernel) remains possible on long runs and is now surfaced by a
+  warning; it was not observed in these short tests.
 - Consequence for validation: physics comparisons must either run
   `-j 1` (exact) or compare distributions statistically. The
   single-threaded benchmark recipes in the justfile now pin `-j 1`.

scripts/physics_list_failure.sh

@@ -0,0 +1,75 @@
+#!/usr/bin/env bash
+# SPDX-FileCopyrightText: 2026 CERN for the benefit of the SHiP Collaboration
+#
+# SPDX-License-Identifier: LGPL-3.0-or-later
+
+# Clean-failure check: an unknown physics_list must make the Geant4 module fail
+# with a catchable, clear error rather than aborting the process. Geant4's
+# G4PhysListFactory::GetReferencePhysList raises a fatal G4Exception (SIGABRT)
+# for an unknown list, so the module validates the name up front and throws a
+# std::runtime_error instead.
+#
+# The module's std::call_once init is sticky: the first failure is captured and
+# rethrown on every later simulate call, so phlex drains the events already in
+# flight, reports the error, and exits — instead of a retry constructing a
+# second G4MTRunManager and hitting the fatal "G4RunManager constructed twice"
+# abort. We drive a few events with -j > 1 so several events pass through the
+# sticky error, and check below that none of them triggered that abort.
+set -uo pipefail
+
+workdir=$(mktemp -d)
+trap 'rm -rf "$workdir"' EXIT
+
+cat >"$workdir/bad_physics.jsonnet" <<'EOF'
+{
+  driver: {
+    cpp: 'generate_layers',
+    layers: { event: { total: 8 } },
+  },
+  sources: {
+    geometry: { cpp: 'geometry_builtin_provider' },
+    field: { cpp: 'field_null_provider' },
+    gun: {
+      cpp: 'particle_gun_source',
+      pdg: 13,
+      p_min: 20.0,
+      p_max: 20.0,
+      max_theta: 0.0,
+      vertex_z: -2000.0,
+    },
+  },
+  modules: {
+    geant4: {
+      cpp: 'geant4_module',
+      physics_list: 'NO_SUCH_LIST',
+      concurrency: 4,
+    },
+    output: { cpp: 'sim_output_module', mode: 'noop' },
+  },
+}
+EOF
+
+out=$(phlex -c <(jsonnet "$workdir/bad_physics.jsonnet") -j 4 2>&1)
+rc=$?
+
+if [ "$rc" -eq 0 ]; then
+  echo "FAIL: expected failure on unknown physics list, but phlex succeeded"
+  echo "$out"
+  exit 1
+fi
+
+if ! grep -q "Unknown physics list" <<<"$out"; then
+  echo "FAIL: phlex failed (exit $rc) but not with the clean 'Unknown physics list' error"
+  echo "$out"
+  exit 1
+fi
+
+# Re-constructing the run manager aborts via G4Exception (SIGABRT -> exit 134)
+# with "constructed twice"; the sticky-failure path never does.
+if [ "$rc" -eq 134 ] || grep -qi "constructed twice" <<<"$out"; then
+  echo "FAIL: init aborted the run manager instead of failing cleanly (exit $rc)"
+  echo "$out"
+  exit 1
+fi
+
+echo "physics-list check passed: unknown list fails cleanly, no run-manager abort"

src/geant4_module.cpp

@@ -123,6 +123,9 @@ class Geant4Sim {
 
   ~Geant4Sim() {
     if (!master_constructed_) return;
+    if (int kernels = built_kernels_.load(); kernels > 0)
+      spdlog::info("geant4: {} worker kernel(s) built for {} configured slots",
+                   kernels, cfg_.concurrency);
     // 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
@@ -287,6 +290,17 @@ class Geant4Sim {
             "' does not exist");
     }
 
+    // Validate the physics list before constructing anything. Like export_gdml
+    // above, GetReferencePhysList raises a fatal G4Exception for an unknown
+    // list, so query IsReferencePhysList here — before detector_ or the run
+    // manager exist — to fail with a catchable, clear error instead of aborting
+    // the process. Failing this early also keeps the failure cheap: nothing has
+    // been built yet. G4PhysListFactory only consults a static name table (it
+    // creates no geometry), so validating on the calling thread is safe.
+    G4PhysListFactory phys_factory;
+    if (!phys_factory.IsReferencePhysList(cfg_.physics_list))
+      throw std::runtime_error("Unknown physics list: " + cfg_.physics_list);
+
     field_ = field;  // keep alive for the G4 run
     detector_ = new ConfigurableDetectorConstruction(
         *geo, *field, cfg_.sd_mode, cfg_.ke_threshold, cfg_.regions);
@@ -306,14 +320,11 @@ class Geant4Sim {
       rm->SetNumberOfThreads(cfg_.concurrency);
       rm->SetUserInitialization(detector_);
 
-      // The run manager is constructed first so its G4Exception handler is
-      // installed: G4PhysListFactory raises a fatal G4Exception (and aborts)
-      // for an unknown list rather than returning null, so this lookup is not
-      // retryable regardless of ordering.
+      // Physics list already validated in init_master before the run manager
+      // was constructed, so GetReferencePhysList won't hit its unknown-list
+      // abort.
       G4PhysListFactory factory;
       auto* physics = factory.GetReferencePhysList(cfg_.physics_list);
-      if (!physics)
-        throw std::runtime_error("Unknown physics list: " + cfg_.physics_list);
       rm->SetUserInitialization(physics);
 
       rm->SetUserInitialization(new DirectActionInit());
@@ -351,6 +362,17 @@ class Geant4Sim {
   void init_worker() {
     AEGIR_TRACE_EVENT("g4", "init_worker");
     int id = next_thread_id_.fetch_add(1);
+    // TBB does not promise that the same OS threads serve this transform
+    // for the whole run, so more kernels than configured slots can appear
+    // (each new thread that ever runs simulate builds one). Geant4 11
+    // tolerates thread ids beyond SetNumberOfThreads in this direct-
+    // injection flow, but each extra kernel costs memory and a fresh RNG
+    // stream — make it visible instead of silent.
+    if (id >= cfg_.concurrency)
+      spdlog::warn(
+          "geant4: initialising worker kernel #{} beyond the {} configured "
+          "slots (TBB thread churn)",
+          id + 1, cfg_.concurrency);
     AEGIR_TRACE_THREAD_NAME("g4_worker_" + std::to_string(id));
     G4Threading::G4SetThreadId(id);
     G4WorkerThread::BuildGeometryAndPhysicsVector();
@@ -371,6 +393,11 @@ class Geant4Sim {
 
     tl_kernel->RunInitialization();
     G4StateManager::GetStateManager()->SetNewState(G4State_GeomClosed);
+
+    // Count only here, once the kernel is fully built: next_thread_id_ is
+    // bumped on entry (to allocate the thread id) and would overcount a worker
+    // whose init_worker threw partway through.
+    built_kernels_.fetch_add(1);
   }
 
   Geant4SimConfig cfg_;
@@ -384,6 +411,9 @@ class Geant4Sim {
   ConfigurableDetectorConstruction* detector_ = nullptr;  // owned by G4
   std::shared_ptr<ship::IFieldSource> field_;             // outlives G4 run
   std::atomic<int> next_thread_id_{0};
+  // Kernels that finished init_worker successfully (unlike next_thread_id_,
+  // which counts allocated ids). Reported in the shutdown summary.
+  std::atomic<int> built_kernels_{0};
   std::atomic<int> 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.