Phase 1: derive TMS geometry bounds from a GDML survey instead of hardcoded constants

[SFBayLaser]

Part of a proposed 3-phase restructuring to prepare dune-tms for real (non-MC) data (discussed in #nd_muon_spectrometer_code). This is Phase 1. Heads-up issue: #291.

Problem: several places (TMS_Constants.h, config, various bounds checks) hardcoded the TMS geometry's bounding box/plane/module counts for one specific geometry, and silently drift out of sync with any other GDML.

Fix: added a one-time geometry survey (TMS_Geom::SurveyGeometry()) that derives these values from whatever GDML is actually loaded, and pointed the existing hardcoded bounds checks (Kalman fit bounds, hit-rejection filter, Hough seed bounds, TMS_Bar plane/bar validation, event viewer bounds) at it instead.

Also includes: reconciliation with PR #280's node-path-lookup BarNumber scheme (adopted #280's approach for BarNumber, kept our survey-based validation as a complement), and a follow-up fix for a PlaneNumber collision bug this surfaced -- raw ROOT node numbers aren't globally unique across all physical planes in some geometries (81 distinct planes vs only 51 raw node values in one test geometry), which silently broke A* pathfinding's plane-ordering assumption in TMS_Reco.cpp. Fixed by using the same z-ordered lookup table BarNumber already relies on.

Verification: build clean in the SL7/Apptainer container; full-spill regression run confirms RecoHitPlane/TrueHitPlane now span the correct 0-81 range with zero negative sentinels, and nHits/ntracks/NTrueHits deltas vs a pristine-main baseline collapsed to <1% after the PlaneNumber fix (down from double-digit-percent swings before it).

Closes #291.

[SFBayLaser]

I should note that the 4 PRs will be "breaking changes" so I would not expect things to "work" without all four PRs taken together.

[Copilot]

Pull request overview

This PR is Phase 1 of making dune-tms geometry-driven by deriving TMS bounds and (some) indexing constraints from the loaded GDML via a one-time geometry survey, replacing several hardcoded geometry constants and volume-name strings that could drift out of sync across GDML variants.

Changes:

• Added TMS_Geom::SurveyGeometry() (and build integration) to walk the GDML node tree once and derive the scintillator-bar bounding box plus surveyed plane/module identity sets.
• Switched multiple reconstruction/viewer bounds checks (Kalman, hit cleaning, event viewer, etc.) from TMS_Const::* hardcoded bounds to TMS_Geom survey-derived accessors.
• Moved geometry volume-name substrings from TMS_Constants.h into config/TMS_Default_Config.toml and plumbed them through TMS_Manager for config-driven geometry traversal.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/TMS_Reco.cpp	Updates hit cleaning / Hough bounds to use TMS_Geom survey-derived TMS Z/X extents.
src/TMS_Manager.h	Adds getters/storage for config-driven geometry volume-name substrings.
src/TMS_Manager.cpp	Loads new [Geometry.VolumeNames] TOML keys into TMS_Manager.
src/TMS_Kalman.cpp	Uses surveyed TMS start/end bounds for inside-box checks and Kalman bounds validation.
src/TMS_Geom.h	Introduces survey layout struct, survey-backed TMS bounds accessors, and volume-name usage via TMS_Manager.
src/TMS_Geom.cpp	Implements the recursive GDML node walk and survey reporting / safety cap behavior.
src/TMS_EventViewer.cpp	Switches viewer axis bounds and thin/thick line extents to TMS_Geom bounds.
src/TMS_Event.cpp	Replaces hardcoded volume-name filtering with config-driven TMS_Manager volume-name substrings.
src/TMS_Constants.h	Removes compiled-in volume-name strings (now TOML-driven) and documents the change.
src/TMS_Bar.h	Updates GetBarNumber() comment to reflect node-path lookup semantics.
src/TMS_Bar.cpp	Switches volume-name matching to TOML-driven values; updates bar/module validation to use survey results when available.
src/Makefile	Adds TMS_Geom.o to build now that TMS_Geom.cpp exists.
config/TMS_Default_Config.toml	Adds [Geometry.VolumeNames] defaults for config-driven geometry traversal.

Suppressed comments (2)

src/TMS_Bar.cpp:262

• Same issue as above: throw; outside a catch block will terminate the program, and return false; is unreachable. Throw a concrete exception (or return an error) so failures are diagnosable.

    if (!tmsGeom.IsSurveyedModuleNumber(GlobalBarNumber)) {
      std::cerr << "Global bar number " << GlobalBarNumber << " was not among the "
        << tmsGeom.GetNDistinctModuleNumbers() << " distinct module numbers found by the geometry survey." << std::endl;
      throw;
      return false;
    }

src/TMS_Bar.cpp:269

• Same issue as above: throw; outside a catch block will terminate the program, and return false; is unreachable. Throw a concrete exception so the failure mode is actionable.

    if (GlobalBarNumber >= TMS_Const::nModules || GlobalBarNumber < 0) {
      std::cerr << "Global bar number does not agree with expectation of between 0 to " << TMS_Const::nModules << std::endl;
      std::cerr << "Has the geometry been updated without updating the geometry constants in TMS_Constants.h?" << std::endl;
      throw;
      return false;
    }

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/TMS_Bar.cpp:281

• This plane-range failure message still points to updating TMS_Constants.h, but PlaneNumber is now derived from TMS_Geom’s z-ordered lookup-table scheme. Updating the message to reference the lookup/config makes it much easier to debug real geometry mismatches.

  if (PlaneNumber >= expected_nplanes || PlaneNumber < 0) {
    std::cerr << "Plane number does not agree with expectation of between 0 to " << expected_nplanes << std::endl;
    std::cerr << "Has the geometry been updated without updating the geometry constants in TMS_Constants.h?" << std::endl;
    throw std::runtime_error("TMS_Bar::CheckBar(): plane number out of the expected range");

src/TMS_Bar.cpp:248

• This error message suggests updating TMS_Constants.h, but BarNumber comes from the geometry bar lookup (node-path map). When BarNumber < 0, the likely actionable fix is checking the loaded GDML and the config-driven volume-name substrings, not recompiling constants; the current message can send users in the wrong direction.

This issue also appears on line 278 of the same file.

    std::cerr << "Bar number was not found in the geometry bar lookup." << std::endl;
    std::cerr << "Has the geometry been updated without updating the geometry constants in TMS_Constants.h?" << std::endl;
    std::cout << "Bar number: " << BarNumber << std::endl;
    throw std::runtime_error("TMS_Bar::CheckBar(): bar number not found in the geometry bar lookup");

src/TMS_Manager.cpp:127

• The exception text has a small grammar issue: “missing (or has a malformed)” reads awkwardly and can be simplified without changing meaning.

    throw std::runtime_error(
        "TMS_Manager: config '" + filename + "' is missing (or has a malformed) "
        "[Geometry.VolumeNames] section. This section was added by the geometry-survey "

[SFBayLaser]

It seems the issue found in TMS_Bar.cpp was, in the end, the source of the "non-determinisn" attributed to the Time Slicer. So this has been resolved going forward.