From 32a1dd01a071efe62eaada9f4ce4466d695c7862 Mon Sep 17 00:00:00 2001 From: Matei Climescu Date: Thu, 2 Jul 2026 00:03:37 +0200 Subject: [PATCH 1/4] feat(apps): build individual subsystems from build_geometry docs(readme): document self-registration and single-subsystem builds --- Geometry]: | 0 README.md | 53 ++++- apps/build_geometry.cpp | 81 ++++++-- include/SHiPGeometry/SubsystemRegistry.h | 93 +++++++++ src/CMakeLists.txt | 7 + src/SHiPGeometry.cpp | 181 +++++++++--------- .../include/Calorimeter/CalorimeterFactory.h | 6 + .../Calorimeter/src/CalorimeterFactory.cpp | 3 + .../Cavern/include/Cavern/CavernFactory.h | 7 + subsystems/Cavern/src/CavernFactory.cpp | 3 + .../include/DecayVolume/DecayVolumeFactory.h | 7 + .../DecayVolume/src/DecayVolumeFactory.cpp | 3 + .../Magnet/include/Magnet/MagnetFactory.h | 6 + subsystems/Magnet/src/MagnetFactory.cpp | 3 + .../include/MuonShield/MuonShieldFactory.h | 7 + .../MuonShield/src/MuonShieldFactory.cpp | 3 + .../NeutrinoDetectorFactory.h | 7 + .../src/NeutrinoDetectorFactory.cpp | 3 + .../Target/include/Target/TargetFactory.h | 7 + subsystems/Target/src/TargetFactory.cpp | 3 + .../TimingDetector/TimingDetectorFactory.h | 6 + .../src/TimingDetectorFactory.cpp | 3 + .../include/Trackers/TrackersFactory.h | 6 + subsystems/Trackers/src/TrackersFactory.cpp | 3 + .../UpstreamTagger/UpstreamTaggerFactory.h | 7 + .../src/UpstreamTaggerFactory.cpp | 3 + 26 files changed, 391 insertions(+), 120 deletions(-) create mode 100644 Geometry]: create mode 100644 include/SHiPGeometry/SubsystemRegistry.h diff --git a/Geometry]: b/Geometry]: new file mode 100644 index 0000000..e69de29 diff --git a/README.md b/README.md index c51ebe2..f0a7b6e 100644 --- a/README.md +++ b/README.md @@ -71,10 +71,22 @@ ctest --test-dir build # Build the complete SHiP geometry ./build/apps/build_geometry [output_file.db] +# Build a single subsystem on its own +./build/apps/build_geometry [output_file.db] + +# List the available subsystem names +./build/apps/build_geometry --list + # View in gmex gmex output_file.db ``` +With no subsystem named, the complete detector is built. Naming a subsystem +(as spelled by `--list`, e.g. `Calorimeter`) builds just that subsystem in its +own local frame. A token ending in `.db` is taken as the output file; the +default is `ship_geometry.db` for the full build, or `.db` for a single +subsystem. + ### Installing ```bash @@ -96,20 +108,39 @@ GeoModelTools automatically. ### Factory Pattern -Each subsystem is implemented as a factory class: +Each subsystem is a self-contained factory class that builds its geometry, +describes where it belongs, and registers itself: ```cpp class FooFactory { public: explicit FooFactory(SHiPMaterials& materials); GeoPhysVol* build(); + + // Self-description consumed by the assembler: + // name, tree node, id, placement (x/y/z mm), and whether it is the world. + static SubsystemDescriptor descriptor() { + return {"Foo", "/SHiP/foo", 42, 0.0, 0.0, 12345.0, /*isWorld=*/false}; + } private: SHiPMaterials& m_materials; }; ``` -`SHiPGeometryBuilder::build()` orchestrates all factories, creating the world -volume (Cavern) and placing each subsystem at its global z-position. +with a single registration line in the factory's `.cpp` (inside +`namespace SHiPGeometry`): + +```cpp +REGISTER_SUBSYSTEM(FooFactory) +``` + +The assembler names no subsystem. `assembleGeometry()` — also reachable via +the unchanged `SHiPGeometryBuilder::build()` — iterates the registry, builds +the world (the subsystem whose descriptor sets `isWorld`, i.e. the Cavern), +and places every other registered subsystem at its declared position, ordered +by `(z, id)`. A single subsystem can be built on its own, in its local frame, +with `buildSubsystem("Foo")`. The registry, descriptor type, and macro live in +`include/SHiPGeometry/SubsystemRegistry.h`. ### Materials @@ -128,13 +159,19 @@ To add a new material, edit `src/SHiPMaterials.cpp`: ## Adding or Modifying a Subsystem 1. **Header**: `subsystems//include//Factory.h` — declare - the factory class with dimension constants as `static constexpr` members + the factory class with dimension constants as `static constexpr` members, + plus `static SubsystemDescriptor descriptor()` returning its name, tree + node, id, and placement 2. **Implementation**: `subsystems//src/Factory.cpp` — implement `build()` using GeoModel primitives (`GeoBox`, `GeoTubs`, `GeoLogVol`, `GeoPhysVol`, `GeoTransform`, etc.) -3. **Registration**: add a `build()` + placement call in - `src/SHiPGeometry.cpp` (`SHiPGeometryBuilder::build()`) -4. **CMake**: add sources/headers to `subsystems//CMakeLists.txt` +3. **Registration**: add one line — `REGISTER_SUBSYSTEM(Factory)` — at + the end of the factory `.cpp`, inside `namespace SHiPGeometry`. The + subsystem registers itself; **do not** edit `src/SHiPGeometry.cpp`, which + names no subsystem. +4. **CMake**: add sources/headers to `subsystems//CMakeLists.txt`, and + add the new library to the `--whole-archive` block in `src/CMakeLists.txt` + so its self-registration initializer is not dropped by the linker 5. **Docs**: update the subsystem `README.md` with geometry tree, materials, and status @@ -142,7 +179,7 @@ To add a new material, edit `src/SHiPMaterials.cpp`: ``` geometry/ -├── include/SHiPGeometry/ # Public headers (SHiPGeometry, SHiPMaterials) +├── include/SHiPGeometry/ # Public headers (SHiPGeometry, SHiPMaterials, SubsystemRegistry) ├── src/ # Core implementation ├── subsystems/ # Detector subsystem factories │ ├── Cavern/ diff --git a/apps/build_geometry.cpp b/apps/build_geometry.cpp index b1ae5bb..2683591 100644 --- a/apps/build_geometry.cpp +++ b/apps/build_geometry.cpp @@ -1,7 +1,20 @@ // SPDX-License-Identifier: LGPL-3.0-or-later // Copyright (C) CERN for the benefit of the SHiP Collaboration +// +// Build the SHiP GeoModel geometry and serialise it to a GeoModel SQLite (.db). +// +// Usage: +// build_geometry # full detector -> ship_geometry.db +// build_geometry out.db # full detector -> out.db +// build_geometry Calorimeter # just that subsystem -> Calorimeter.db +// build_geometry Calorimeter c.db # just that subsystem -> c.db +// build_geometry --list # list available subsystems +// +// A token ending in ".db" is the output file; any other token names a +// subsystem. With no subsystem named, the complete detector is built. #include "SHiPGeometry/SHiPGeometry.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include #include @@ -9,39 +22,73 @@ #include #include +#include +#include + +namespace { +bool endsWith(const std::string& s, const std::string& suffix) { + return s.size() >= suffix.size() && + s.compare(s.size() - suffix.size(), suffix.size(), suffix) == 0; +} +} // namespace int main(int argc, char* argv[]) { - std::string outputFile = "ship_geometry.db"; - if (argc > 1) { - outputFile = argv[1]; + std::string outputFile; + std::vector names; // requested subsystem(s) + + for (int i = 1; i < argc; ++i) { + const std::string arg = argv[i]; + if (arg == "--list") { + for (const auto& n : SHiPGeometry::subsystemNames()) std::cout << n << "\n"; + return 0; + } + if (endsWith(arg, ".db")) { + outputFile = arg; + } else { + names.push_back(arg); + } } - std::cout << "Building SHiP geometry..." << std::endl; + GeoVPhysVol* geometry = nullptr; + std::string label; - SHiPGeometry::SHiPGeometryBuilder builder; - GeoPhysVol* world = builder.build(); + if (names.empty()) { + // Default: the complete detector (unchanged behaviour). + SHiPGeometry::SHiPGeometryBuilder builder; + geometry = builder.build(); + label = "full SHiP geometry"; + if (outputFile.empty()) outputFile = "ship_geometry.db"; + } else if (names.size() == 1) { + // A single subsystem, on its own (local frame). + try { + geometry = SHiPGeometry::buildSubsystem(names[0]); + } catch (const std::exception& e) { + std::cerr << "Error: " << e.what() << "\nAvailable subsystems:\n"; + for (const auto& n : SHiPGeometry::subsystemNames()) std::cerr << " " << n << "\n"; + return 1; + } + label = names[0]; + if (outputFile.empty()) outputFile = names[0] + ".db"; + } else { + std::cerr << "Error: build at most one subsystem at a time (got " << names.size() + << ").\n"; + return 1; + } - if (!world) { - std::cerr << "Error: Geometry not yet implemented" << std::endl; + if (!geometry) { + std::cerr << "Error: geometry is null (not yet implemented?)." << std::endl; return 1; } - // Remove existing output file if (std::filesystem::exists(outputFile)) { std::filesystem::remove(outputFile); } - std::cout << "Writing geometry to " << outputFile << std::endl; - + std::cout << "Writing " << label << " to " << outputFile << std::endl; GMDBManager db(outputFile); GeoModelIO::WriteGeoModel writer(db); - - // Traverse the geometry tree - world->exec(&writer); - - // Save to database + geometry->exec(&writer); writer.saveToDB(); - std::cout << "Done." << std::endl; return 0; } diff --git a/include/SHiPGeometry/SubsystemRegistry.h b/include/SHiPGeometry/SubsystemRegistry.h new file mode 100644 index 0000000..35ec9cf --- /dev/null +++ b/include/SHiPGeometry/SubsystemRegistry.h @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later +// Copyright (C) CERN for the benefit of the SHiP Collaboration + +#pragma once + +#include // complete GeoPhysVol/GeoVPhysVol for the macro's upcast + +#include +#include +#include +#include + +namespace SHiPGeometry { + +class SHiPMaterials; + +/** + * @brief A subsystem's self-description: everything the assembler needs. + * + * This is the "required input to the geometry builder" that each subsystem + * yields about itself. It is a plain data type (no GeoModel dependency). + * Translations are in millimetres from the world origin. + */ +struct SubsystemDescriptor { + const char* name; ///< registry key / CLI name, e.g. "Calorimeter" + const char* node; ///< GeoNameTag, e.g. "/SHiP/calorimeter" + int id; ///< GeoIdentifierTag + double x_mm; ///< placement translation X (mm) + double y_mm; ///< placement translation Y (mm) + double z_mm; ///< placement translation Z, beam direction (mm) + bool isWorld = false; ///< true for the mother/world volume (the cavern) +}; + +/// A registered subsystem: its descriptor plus how to build it (local frame). +struct SubsystemInfo { + SubsystemDescriptor desc; + std::function build; +}; + +/** + * @brief The global subsystem registry. + * + * A Meyers singleton (function-local static in an inline function) so it is + * a single shared instance across all translation units and is guaranteed to + * exist before any static registration runs. No file names any subsystem; + * subsystems add themselves via REGISTER_SUBSYSTEM. + */ +inline std::map& registry() { + static std::map instance; + return instance; +} + +/// Add a subsystem to the registry. Returns true (usable as a static initialiser). +inline bool registerSubsystem(const SubsystemDescriptor& desc, + std::function build) { + registry().emplace(desc.name, SubsystemInfo{desc, std::move(build)}); + return true; +} + +// ── Generic consumers — these name no subsystem ───────────────────────────── + +/// Assemble the world plus a selection of subsystems (empty selection = all), +/// each placed at its own declared position. Returns the world volume. +GeoPhysVol* assembleGeometry(const std::vector& only = {}); + +/// Build a single subsystem on its own, in its local frame (no world, no +/// placement). Throws std::runtime_error if the name is not registered. +GeoVPhysVol* buildSubsystem(const std::string& name); + +/// The names of all registered subsystems (including the world), sorted. +std::vector subsystemNames(); + +} // namespace SHiPGeometry + +/** + * @brief Register a subsystem factory with the global registry. + * + * Placed once in each subsystem's own .cpp (inside namespace SHiPGeometry). + * The factory must expose `static SubsystemDescriptor descriptor()` and be + * constructible from `SHiPMaterials&` with a `build()` returning a volume. + * + * NOTE: because this registration lives in a static library and nothing else + * references it, the subsystem archive must be linked with --whole-archive + * (or an equivalent) so the initialiser is not dropped by the linker. + */ +#define REGISTER_SUBSYSTEM(FACTORY) \ + namespace { \ + const bool FACTORY##_registered = ::SHiPGeometry::registerSubsystem( \ + FACTORY::descriptor(), \ + [](::SHiPGeometry::SHiPMaterials& materials) -> ::GeoVPhysVol* { \ + return FACTORY(materials).build(); \ + }); \ + } diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a7f1e9b..d6ef4c7 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -17,6 +17,12 @@ target_link_libraries( GeoModelCore::GeoModelKernel GeoModelIO::GeoModelRead GeoModelIO::GeoModelWrite + # Force-link the subsystem archives: each registers itself at + # static-init via REGISTER_SUBSYSTEM, and nothing here references + # their symbols, so without --whole-archive the linker would drop + # them and the registry would be empty. (GNU/Clang syntax; for + # CMake >= 3.24 prefer $.) + "-Wl,--whole-archive" Cavern Target MuonShield @@ -27,4 +33,5 @@ target_link_libraries( Magnet TimingDetector Calorimeter + "-Wl,--no-whole-archive" ) diff --git a/src/SHiPGeometry.cpp b/src/SHiPGeometry.cpp index 7c81059..fdff79d 100644 --- a/src/SHiPGeometry.cpp +++ b/src/SHiPGeometry.cpp @@ -1,115 +1,106 @@ // SPDX-License-Identifier: LGPL-3.0-or-later // Copyright (C) CERN for the benefit of the SHiP Collaboration +// +// The assembler. It contains NO subsystem names: every subsystem registers +// itself (see REGISTER_SUBSYSTEM in each subsystem's .cpp) and this file only +// iterates whatever registered. #include "SHiPGeometry/SHiPGeometry.h" -#include "Calorimeter/CalorimeterFactory.h" -#include "Cavern/CavernFactory.h" -#include "DecayVolume/DecayVolumeFactory.h" -#include "Magnet/MagnetFactory.h" -#include "MuonShield/MuonShieldFactory.h" -#include "SHiPGeometry/Placement.h" #include "SHiPGeometry/SHiPMaterials.h" -#include "Target/TargetFactory.h" -#include "TimingDetector/TimingDetectorFactory.h" -#include "Trackers/TrackersFactory.h" -#include "UpstreamTagger/SHiPUBTManager.h" -#include "UpstreamTagger/UpstreamTaggerFactory.h" - -#include "NeutrinoDetector/NeutrinoDetectorFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include +#include +#include #include -#include +#include + +#include +#include +#include +#include namespace SHiPGeometry { -SHiPGeometryBuilder::SHiPGeometryBuilder() = default; -SHiPGeometryBuilder::~SHiPGeometryBuilder() = default; +namespace { -GeoPhysVol* SHiPGeometryBuilder::build() { - // Unit shorthands (GeoModel's native length unit is mm) - constexpr double mm = GeoModelKernelUnits::mm; - constexpr double cm = GeoModelKernelUnits::cm; - constexpr double m = GeoModelKernelUnits::m; +/// Attach a built subsystem volume to the world at its declared placement. +void place(GeoPhysVol* world, GeoVPhysVol* volume, const SubsystemDescriptor& d) { + world->add(new GeoNameTag(d.node)); + world->add(new GeoIdentifierTag(d.id)); + world->add(new GeoTransform(GeoTrf::Translate3D(d.x_mm, d.y_mm, d.z_mm))); + world->add(volume); +} + +} // namespace + +GeoPhysVol* assembleGeometry(const std::vector& only) { + SHiPMaterials materials; + auto& reg = registry(); + + // Validate any requested names up front. + for (const auto& name : only) { + if (reg.find(name) == reg.end()) { + throw std::runtime_error("Unknown subsystem: '" + name + "'"); + } + } + + // The world is whichever registered subsystem marks itself isWorld (cavern). + GeoPhysVol* world = nullptr; + for (auto& entry : reg) { + if (entry.second.desc.isWorld) { + world = dynamic_cast(entry.second.build(materials)); + break; + } + } + if (!world) { + throw std::runtime_error("No world (isWorld) subsystem is registered"); + } + + // Gather the placed subsystems (selection or all), then order them + // deterministically by (z, id) — independent of registration order. + std::vector placed; + for (auto& entry : reg) { + const SubsystemInfo& info = entry.second; + if (info.desc.isWorld) continue; + if (only.empty() || std::find(only.begin(), only.end(), entry.first) != only.end()) { + placed.push_back(&info); + } + } + std::sort(placed.begin(), placed.end(), [](const SubsystemInfo* a, const SubsystemInfo* b) { + if (a->desc.z_mm != b->desc.z_mm) return a->desc.z_mm < b->desc.z_mm; + return a->desc.id < b->desc.id; + }); + + for (const SubsystemInfo* info : placed) { + place(world, info->build(materials), info->desc); + } + return world; +} - // Create central materials manager +GeoVPhysVol* buildSubsystem(const std::string& name) { + auto& reg = registry(); + auto it = reg.find(name); + if (it == reg.end()) { + throw std::runtime_error("Unknown subsystem: '" + name + "'"); + } SHiPMaterials materials; + return it->second.build(materials); +} - // Build the cavern (world volume) - CavernFactory cavernFactory(materials); - GeoPhysVol* world = cavernFactory.build(); - - // Build and place the Target - TargetFactory targetFactory(materials); - GeoPhysVol* target = targetFactory.build(); - - // Position target in world (from GDML: x=0, y=-14.45cm, z=43.25cm) - // Note: These are relative to the cave origin - placeChild(world, target, "/SHiP/target", 1, GeoTrf::Translate3D(0.0, -14.45 * cm, 43.25 * cm)); - - // Build and place MuonShieldArea - // GDML z range: 204–3148.66 cm → centre: 1676.33 cm = 16763.3 mm from world origin - MuonShieldFactory muonShieldFactory(materials); - GeoPhysVol* muonShield = muonShieldFactory.build(); - placeChild(world, muonShield, "/SHiP/muon_shield", 2, - GeoTrf::Translate3D(0.0, 0.0, 16763.3 * mm)); - - // Build and place the Scattering and Neutrino Detector (SND). - // Z: 26.40 to 31.50 m (WARM muon-shield configuration) → centre 28.95 m. - // The SND sits within the downstream end of the muon-shield region, so its - // envelope overlaps the muon-shield container by design (see test_consistency). - NeutrinoDetectorFactory neutrinoDetectorFactory(materials); - GeoPhysVol* neutrinoDetector = neutrinoDetectorFactory.build(); - placeChild(world, neutrinoDetector, "/SHiP/neutrino_detector", 9, - GeoTrf::Translate3D(0.0, 0.0, 28.95 * m)); - - // Build and place UpstreamTagger (sensitive scintillator slab) - // Z: 32.52 to 32.92 m → centre: 32.72 m - SHiPUBTManager ubtManager; - UpstreamTaggerFactory upstreamTaggerFactory(materials); - GeoVPhysVol* upstreamTagger = upstreamTaggerFactory.build(&ubtManager); - placeChild(world, upstreamTagger, "/SHiP/upstream_tagger", 3, - GeoTrf::Translate3D(0.0, 0.0, 32.72 * m)); - - // Build and place DecayVolume - // Z: 32.92 to 83.32 m → centre: 58.12 m - DecayVolumeFactory decayVolumeFactory(materials); - GeoPhysVol* decayVolume = decayVolumeFactory.build(); - placeChild(world, decayVolume, "/SHiP/decay_volume", 4, - GeoTrf::Translate3D(0.0, 0.0, 58.12 * m)); - - // Build and place Trackers (container with 4 stations). - // The factory already handles internal positioning; place the container at - // its centre Z (average of station 1 and 4 centres). - TrackersFactory trackersFactory(materials); - GeoPhysVol* trackers = trackersFactory.build(); - constexpr double trackersCentreZ = (84.07 + 95.07) / 2.0 * m; - placeChild(world, trackers, "/SHiP/trackers", 5, - GeoTrf::Translate3D(0.0, 0.0, trackersCentreZ)); - - // Build and place Magnet - // Z: 87.07 to 92.07 m → centre: 89.57 m - MagnetFactory magnetFactory(materials); - GeoPhysVol* magnet = magnetFactory.build(); - placeChild(world, magnet, "/SHiP/magnet", 6, GeoTrf::Translate3D(0.0, 0.0, 89.57 * m)); - - // Build and place TimingDetector - // Z: 95.902 m (from GDML reference) - TimingDetectorFactory timingDetectorFactory(materials); - GeoPhysVol* timingDetector = timingDetectorFactory.build(); - placeChild(world, timingDetector, "/SHiP/timing_detector", 7, - GeoTrf::Translate3D(0.0, 0.0, 95.902 * m)); - - // Build and place Calorimeter (ECAL + HCAL). - // The layer structure is driven by calo.toml; the outer container dimensions - // and placement are fixed to match the SHiP subsystem envelope. - CalorimeterFactory calorimeterFactory(materials); - GeoPhysVol* calorimeter = calorimeterFactory.build(); - placeChild(world, calorimeter, "/SHiP/calorimeter", 8, - GeoTrf::Translate3D(0.0, 0.0, 98.32 * m)); +std::vector subsystemNames() { + std::vector names; + for (auto& entry : registry()) names.push_back(entry.first); + return names; +} - return world; +// ── Backwards-compatible builder: unchanged interface, delegates to registry ── +SHiPGeometryBuilder::SHiPGeometryBuilder() = default; +SHiPGeometryBuilder::~SHiPGeometryBuilder() = default; + +GeoPhysVol* SHiPGeometryBuilder::build() { + return assembleGeometry({}); } } // namespace SHiPGeometry diff --git a/subsystems/Calorimeter/include/Calorimeter/CalorimeterFactory.h b/subsystems/Calorimeter/include/Calorimeter/CalorimeterFactory.h index d8a1777..41427e0 100644 --- a/subsystems/Calorimeter/include/Calorimeter/CalorimeterFactory.h +++ b/subsystems/Calorimeter/include/Calorimeter/CalorimeterFactory.h @@ -4,6 +4,7 @@ #pragma once #include +#include "SHiPGeometry/SubsystemRegistry.h" #include @@ -32,6 +33,11 @@ class CalorimeterFactory { explicit CalorimeterFactory(SHiPMaterials& materials, std::string configPath = "calo.toml"); ~CalorimeterFactory() = default; + + /// This subsystem's self-description (name, node, id, placement). + static SubsystemDescriptor descriptor() { + return {"Calorimeter", "/SHiP/calorimeter", 8, 0.0, 0.0, 98320.0, false}; + } /** Build and return the calorimeter container volume. */ [[nodiscard]] GeoPhysVol* build(); diff --git a/subsystems/Calorimeter/src/CalorimeterFactory.cpp b/subsystems/Calorimeter/src/CalorimeterFactory.cpp index 1cb2c0f..0e26d4b 100644 --- a/subsystems/Calorimeter/src/CalorimeterFactory.cpp +++ b/subsystems/Calorimeter/src/CalorimeterFactory.cpp @@ -2,6 +2,7 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "Calorimeter/CalorimeterFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "Calorimeter/CaloBarLayer.h" #include "Calorimeter/CaloFibreHPLayer.h" @@ -351,4 +352,6 @@ void CalorimeterFactory::buildStack(GeoPhysVol* container, const CalorimeterConf .incrementGlobalOnAirGap = true}); } +REGISTER_SUBSYSTEM(CalorimeterFactory) + } // namespace SHiPGeometry diff --git a/subsystems/Cavern/include/Cavern/CavernFactory.h b/subsystems/Cavern/include/Cavern/CavernFactory.h index 04c814d..1ff80f6 100644 --- a/subsystems/Cavern/include/Cavern/CavernFactory.h +++ b/subsystems/Cavern/include/Cavern/CavernFactory.h @@ -3,6 +3,8 @@ #pragma once +#include "SHiPGeometry/SubsystemRegistry.h" + #include class GeoPhysVol; @@ -19,6 +21,11 @@ class CavernFactory { explicit CavernFactory(SHiPMaterials& materials); ~CavernFactory() = default; + + /// This subsystem's self-description (name, node, id, placement). + static SubsystemDescriptor descriptor() { + return {"Cavern", "/SHiP/cavern", 0, 0.0, 0.0, 0.0, true}; + } /** * @brief Build the Cavern geometry * @return Pointer to the world physical volume diff --git a/subsystems/Cavern/src/CavernFactory.cpp b/subsystems/Cavern/src/CavernFactory.cpp index 47acc95..912fd09 100644 --- a/subsystems/Cavern/src/CavernFactory.cpp +++ b/subsystems/Cavern/src/CavernFactory.cpp @@ -2,6 +2,7 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "Cavern/CavernFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" @@ -80,4 +81,6 @@ GeoPhysVol* CavernFactory::build() { return m_world; } +REGISTER_SUBSYSTEM(CavernFactory) + } // namespace SHiPGeometry diff --git a/subsystems/DecayVolume/include/DecayVolume/DecayVolumeFactory.h b/subsystems/DecayVolume/include/DecayVolume/DecayVolumeFactory.h index 91a1ee6..02c161c 100644 --- a/subsystems/DecayVolume/include/DecayVolume/DecayVolumeFactory.h +++ b/subsystems/DecayVolume/include/DecayVolume/DecayVolumeFactory.h @@ -3,6 +3,8 @@ #pragma once +#include "SHiPGeometry/SubsystemRegistry.h" + #include class GeoPhysVol; @@ -29,6 +31,11 @@ class DecayVolumeFactory { explicit DecayVolumeFactory(SHiPMaterials& materials, std::string configPath = "sbt.toml"); ~DecayVolumeFactory() = default; + + /// This subsystem's self-description (name, node, id, placement). + static SubsystemDescriptor descriptor() { + return {"DecayVolume", "/SHiP/decay_volume", 4, 0.0, 0.0, 58120.0, false}; + } /// Build the DecayVolume geometry; returns the air container. [[nodiscard]] GeoPhysVol* build(); diff --git a/subsystems/DecayVolume/src/DecayVolumeFactory.cpp b/subsystems/DecayVolume/src/DecayVolumeFactory.cpp index 9b2c2af..be202b9 100644 --- a/subsystems/DecayVolume/src/DecayVolumeFactory.cpp +++ b/subsystems/DecayVolume/src/DecayVolumeFactory.cpp @@ -2,6 +2,7 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "DecayVolume/DecayVolumeFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "DecayVolume/SBTConfig.h" #include "DecayVolume/SBTSensorBuilder.h" @@ -121,4 +122,6 @@ GeoPhysVol* DecayVolumeFactory::build() { return container; } +REGISTER_SUBSYSTEM(DecayVolumeFactory) + } // namespace SHiPGeometry diff --git a/subsystems/Magnet/include/Magnet/MagnetFactory.h b/subsystems/Magnet/include/Magnet/MagnetFactory.h index a735212..46415f0 100644 --- a/subsystems/Magnet/include/Magnet/MagnetFactory.h +++ b/subsystems/Magnet/include/Magnet/MagnetFactory.h @@ -4,6 +4,7 @@ #pragma once #include +#include "SHiPGeometry/SubsystemRegistry.h" #include @@ -26,6 +27,11 @@ class MagnetFactory { explicit MagnetFactory(SHiPMaterials& materials); ~MagnetFactory() = default; + + /// This subsystem's self-description (name, node, id, placement). + static SubsystemDescriptor descriptor() { + return {"Magnet", "/SHiP/magnet", 6, 0.0, 0.0, 89570.0, false}; + } /** * @brief Build the Magnet geometry * @return Pointer to the physical volume diff --git a/subsystems/Magnet/src/MagnetFactory.cpp b/subsystems/Magnet/src/MagnetFactory.cpp index 7a2cd6c..f1fa35e 100644 --- a/subsystems/Magnet/src/MagnetFactory.cpp +++ b/subsystems/Magnet/src/MagnetFactory.cpp @@ -2,6 +2,7 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "Magnet/MagnetFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" @@ -120,4 +121,6 @@ GeoPhysVol* MagnetFactory::createVerticalConnector(const std::string& name) { return new GeoPhysVol(connectorLog); } +REGISTER_SUBSYSTEM(MagnetFactory) + } // namespace SHiPGeometry diff --git a/subsystems/MuonShield/include/MuonShield/MuonShieldFactory.h b/subsystems/MuonShield/include/MuonShield/MuonShieldFactory.h index 366aad8..25246e1 100644 --- a/subsystems/MuonShield/include/MuonShield/MuonShieldFactory.h +++ b/subsystems/MuonShield/include/MuonShield/MuonShieldFactory.h @@ -4,6 +4,7 @@ #pragma once #include +#include "SHiPGeometry/SubsystemRegistry.h" class GeoPhysVol; @@ -27,6 +28,12 @@ class MuonShieldFactory { [[nodiscard]] GeoPhysVol* build(); + /// This subsystem's self-description (name, node, id, placement). + static SubsystemDescriptor descriptor() { + return {"MuonShield", "/SHiP/muon_shield", 2, 0.0, 0.0, 16763.3, false}; + } + GeoPhysVol* build(); + private: SHiPMaterials& m_materials; diff --git a/subsystems/MuonShield/src/MuonShieldFactory.cpp b/subsystems/MuonShield/src/MuonShieldFactory.cpp index f77bd59..8658771 100644 --- a/subsystems/MuonShield/src/MuonShieldFactory.cpp +++ b/subsystems/MuonShield/src/MuonShieldFactory.cpp @@ -2,6 +2,7 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "MuonShield/MuonShieldFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" @@ -181,4 +182,6 @@ GeoPhysVol* MuonShieldFactory::build() { return areaPhys; } +REGISTER_SUBSYSTEM(MuonShieldFactory) + } // namespace SHiPGeometry diff --git a/subsystems/NeutrinoDetector/include/NeutrinoDetector/NeutrinoDetectorFactory.h b/subsystems/NeutrinoDetector/include/NeutrinoDetector/NeutrinoDetectorFactory.h index 012ce6f..ec6e2ca 100644 --- a/subsystems/NeutrinoDetector/include/NeutrinoDetector/NeutrinoDetectorFactory.h +++ b/subsystems/NeutrinoDetector/include/NeutrinoDetector/NeutrinoDetectorFactory.h @@ -3,6 +3,8 @@ #pragma once +#include "SHiPGeometry/SubsystemRegistry.h" + class GeoPhysVol; namespace SHiPGeometry { @@ -49,6 +51,11 @@ class NeutrinoDetectorFactory { /// Defaulted destructor. ~NeutrinoDetectorFactory() = default; + + /// This subsystem's self-description (name, node, id, placement). + static SubsystemDescriptor descriptor() { + return {"NeutrinoDetector", "/SHiP/neutrino_detector", 9, 0.0, 0.0, 28950.0, false}; + } /// Build the SND geometry; returns the air container. [[nodiscard]] GeoPhysVol* build(); diff --git a/subsystems/NeutrinoDetector/src/NeutrinoDetectorFactory.cpp b/subsystems/NeutrinoDetector/src/NeutrinoDetectorFactory.cpp index c617e67..aed2df3 100644 --- a/subsystems/NeutrinoDetector/src/NeutrinoDetectorFactory.cpp +++ b/subsystems/NeutrinoDetector/src/NeutrinoDetectorFactory.cpp @@ -2,6 +2,7 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "NeutrinoDetector/NeutrinoDetectorFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" @@ -291,4 +292,6 @@ GeoPhysVol* NeutrinoDetectorFactory::build() { return containerPhys; } +REGISTER_SUBSYSTEM(NeutrinoDetectorFactory) + } // namespace SHiPGeometry diff --git a/subsystems/Target/include/Target/TargetFactory.h b/subsystems/Target/include/Target/TargetFactory.h index 3f1a29d..576561f 100644 --- a/subsystems/Target/include/Target/TargetFactory.h +++ b/subsystems/Target/include/Target/TargetFactory.h @@ -3,6 +3,8 @@ #pragma once +#include "SHiPGeometry/SubsystemRegistry.h" + #include #include @@ -29,6 +31,11 @@ class TargetFactory { explicit TargetFactory(SHiPMaterials& materials); ~TargetFactory() = default; + + /// This subsystem's self-description (name, node, id, placement). + static SubsystemDescriptor descriptor() { + return {"Target", "/SHiP/target", 1, 0.0, -144.5, 432.5, false}; + } /** * @brief Build the Target geometry * @return Pointer to the target_vacuum_box physical volume diff --git a/subsystems/Target/src/TargetFactory.cpp b/subsystems/Target/src/TargetFactory.cpp index 38c515c..e8d816e 100644 --- a/subsystems/Target/src/TargetFactory.cpp +++ b/subsystems/Target/src/TargetFactory.cpp @@ -2,6 +2,7 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "Target/TargetFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" @@ -239,4 +240,6 @@ GeoPhysVol* TargetFactory::createHeVolume() { return heVolumePhys; } +REGISTER_SUBSYSTEM(TargetFactory) + } // namespace SHiPGeometry diff --git a/subsystems/TimingDetector/include/TimingDetector/TimingDetectorFactory.h b/subsystems/TimingDetector/include/TimingDetector/TimingDetectorFactory.h index e51bfdc..583ab31 100644 --- a/subsystems/TimingDetector/include/TimingDetector/TimingDetectorFactory.h +++ b/subsystems/TimingDetector/include/TimingDetector/TimingDetectorFactory.h @@ -4,6 +4,7 @@ #pragma once #include +#include "SHiPGeometry/SubsystemRegistry.h" class GeoPhysVol; @@ -26,6 +27,11 @@ class TimingDetectorFactory { explicit TimingDetectorFactory(SHiPMaterials& materials); ~TimingDetectorFactory() = default; + + /// This subsystem's self-description (name, node, id, placement). + static SubsystemDescriptor descriptor() { + return {"TimingDetector", "/SHiP/timing_detector", 7, 0.0, 0.0, 95902.0, false}; + } /** Build the TimingDetector geometry and return the container volume. */ [[nodiscard]] GeoPhysVol* build(); diff --git a/subsystems/TimingDetector/src/TimingDetectorFactory.cpp b/subsystems/TimingDetector/src/TimingDetectorFactory.cpp index 01b7496..37b2889 100644 --- a/subsystems/TimingDetector/src/TimingDetectorFactory.cpp +++ b/subsystems/TimingDetector/src/TimingDetectorFactory.cpp @@ -2,6 +2,7 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "TimingDetector/TimingDetectorFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" #include "TimingDetector/SHiPTimingDetInterface.h" @@ -55,4 +56,6 @@ GeoPhysVol* TimingDetectorFactory::build() { return containerPhys; } +REGISTER_SUBSYSTEM(TimingDetectorFactory) + } // namespace SHiPGeometry diff --git a/subsystems/Trackers/include/Trackers/TrackersFactory.h b/subsystems/Trackers/include/Trackers/TrackersFactory.h index efc4708..1579618 100644 --- a/subsystems/Trackers/include/Trackers/TrackersFactory.h +++ b/subsystems/Trackers/include/Trackers/TrackersFactory.h @@ -4,6 +4,7 @@ #pragma once #include +#include "SHiPGeometry/SubsystemRegistry.h" #include #include @@ -40,6 +41,11 @@ class TrackersFactory { explicit TrackersFactory(SHiPMaterials& materials); ~TrackersFactory() = default; + + /// This subsystem's self-description (name, node, id, placement). + static SubsystemDescriptor descriptor() { + return {"Trackers", "/SHiP/trackers", 5, 0.0, 0.0, 89570.0, false}; + } /** * @brief Build the Trackers geometry. * @return Pointer to the container volume holding all 4 stations. diff --git a/subsystems/Trackers/src/TrackersFactory.cpp b/subsystems/Trackers/src/TrackersFactory.cpp index 483352d..f494756 100644 --- a/subsystems/Trackers/src/TrackersFactory.cpp +++ b/subsystems/Trackers/src/TrackersFactory.cpp @@ -2,6 +2,7 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "Trackers/TrackersFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" @@ -314,4 +315,6 @@ GeoPhysVol* TrackersFactory::buildTrackerMagnet() { return phys; } +REGISTER_SUBSYSTEM(TrackersFactory) + } // namespace SHiPGeometry diff --git a/subsystems/UpstreamTagger/include/UpstreamTagger/UpstreamTaggerFactory.h b/subsystems/UpstreamTagger/include/UpstreamTagger/UpstreamTaggerFactory.h index ae4c8be..fb8c253 100644 --- a/subsystems/UpstreamTagger/include/UpstreamTagger/UpstreamTaggerFactory.h +++ b/subsystems/UpstreamTagger/include/UpstreamTagger/UpstreamTaggerFactory.h @@ -3,6 +3,8 @@ #pragma once +#include "SHiPGeometry/SubsystemRegistry.h" + class GeoVPhysVol; namespace SHiPGeometry { @@ -52,6 +54,11 @@ class UpstreamTaggerFactory { explicit UpstreamTaggerFactory(SHiPMaterials& materials); ~UpstreamTaggerFactory() = default; + + /// This subsystem's self-description (name, node, id, placement). + static SubsystemDescriptor descriptor() { + return {"UpstreamTagger", "/SHiP/upstream_tagger", 3, 0.0, 0.0, 32720.0, false}; + } /** * @brief Build the UpstreamTagger geometry. * @param manager Optional manager to register the container tree-top; may be null. diff --git a/subsystems/UpstreamTagger/src/UpstreamTaggerFactory.cpp b/subsystems/UpstreamTagger/src/UpstreamTaggerFactory.cpp index bcb3ddf..daf1f7f 100644 --- a/subsystems/UpstreamTagger/src/UpstreamTaggerFactory.cpp +++ b/subsystems/UpstreamTagger/src/UpstreamTaggerFactory.cpp @@ -2,6 +2,7 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "UpstreamTagger/UpstreamTaggerFactory.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" #include "UpstreamTagger/SHiPUBTManager.h" @@ -141,4 +142,6 @@ GeoVPhysVol* UpstreamTaggerFactory::build(SHiPUBTManager* manager) { return containerPhys; } +REGISTER_SUBSYSTEM(UpstreamTaggerFactory) + } // namespace SHiPGeometry From 274d1cbfcdc8ec63d643a163361993e929315f78 Mon Sep 17 00:00:00 2001 From: Matei Climescu Date: Thu, 2 Jul 2026 00:16:17 +0200 Subject: [PATCH 2/4] style: apply clang-format --- apps/build_geometry.cpp | 15 ++++++----- include/SHiPGeometry/SubsystemRegistry.h | 27 +++++++++---------- src/CMakeLists.txt | 15 +++++------ src/SHiPGeometry.cpp | 9 ++++--- .../include/Calorimeter/CalorimeterFactory.h | 4 +-- .../Calorimeter/src/CalorimeterFactory.cpp | 2 +- .../Cavern/include/Cavern/CavernFactory.h | 1 - subsystems/Cavern/src/CavernFactory.cpp | 2 +- .../include/DecayVolume/DecayVolumeFactory.h | 1 - .../DecayVolume/src/DecayVolumeFactory.cpp | 2 +- .../Magnet/include/Magnet/MagnetFactory.h | 4 +-- subsystems/Magnet/src/MagnetFactory.cpp | 2 +- .../include/MuonShield/MuonShieldFactory.h | 4 +-- .../MuonShield/src/MuonShieldFactory.cpp | 2 +- .../NeutrinoDetectorFactory.h | 1 - .../src/NeutrinoDetectorFactory.cpp | 2 +- .../Target/include/Target/TargetFactory.h | 1 - subsystems/Target/src/TargetFactory.cpp | 2 +- .../TimingDetector/TimingDetectorFactory.h | 4 +-- .../src/TimingDetectorFactory.cpp | 2 +- .../include/Trackers/TrackersFactory.h | 4 +-- subsystems/Trackers/src/TrackersFactory.cpp | 2 +- .../UpstreamTagger/UpstreamTaggerFactory.h | 1 - .../src/UpstreamTaggerFactory.cpp | 2 +- 24 files changed, 55 insertions(+), 56 deletions(-) diff --git a/apps/build_geometry.cpp b/apps/build_geometry.cpp index 2683591..e126cd8 100644 --- a/apps/build_geometry.cpp +++ b/apps/build_geometry.cpp @@ -39,7 +39,8 @@ int main(int argc, char* argv[]) { for (int i = 1; i < argc; ++i) { const std::string arg = argv[i]; if (arg == "--list") { - for (const auto& n : SHiPGeometry::subsystemNames()) std::cout << n << "\n"; + for (const auto& n : SHiPGeometry::subsystemNames()) + std::cout << n << "\n"; return 0; } if (endsWith(arg, ".db")) { @@ -57,21 +58,23 @@ int main(int argc, char* argv[]) { SHiPGeometry::SHiPGeometryBuilder builder; geometry = builder.build(); label = "full SHiP geometry"; - if (outputFile.empty()) outputFile = "ship_geometry.db"; + if (outputFile.empty()) + outputFile = "ship_geometry.db"; } else if (names.size() == 1) { // A single subsystem, on its own (local frame). try { geometry = SHiPGeometry::buildSubsystem(names[0]); } catch (const std::exception& e) { std::cerr << "Error: " << e.what() << "\nAvailable subsystems:\n"; - for (const auto& n : SHiPGeometry::subsystemNames()) std::cerr << " " << n << "\n"; + for (const auto& n : SHiPGeometry::subsystemNames()) + std::cerr << " " << n << "\n"; return 1; } label = names[0]; - if (outputFile.empty()) outputFile = names[0] + ".db"; + if (outputFile.empty()) + outputFile = names[0] + ".db"; } else { - std::cerr << "Error: build at most one subsystem at a time (got " << names.size() - << ").\n"; + std::cerr << "Error: build at most one subsystem at a time (got " << names.size() << ").\n"; return 1; } diff --git a/include/SHiPGeometry/SubsystemRegistry.h b/include/SHiPGeometry/SubsystemRegistry.h index 35ec9cf..6fa2b7d 100644 --- a/include/SHiPGeometry/SubsystemRegistry.h +++ b/include/SHiPGeometry/SubsystemRegistry.h @@ -22,13 +22,13 @@ class SHiPMaterials; * Translations are in millimetres from the world origin. */ struct SubsystemDescriptor { - const char* name; ///< registry key / CLI name, e.g. "Calorimeter" - const char* node; ///< GeoNameTag, e.g. "/SHiP/calorimeter" - int id; ///< GeoIdentifierTag - double x_mm; ///< placement translation X (mm) - double y_mm; ///< placement translation Y (mm) - double z_mm; ///< placement translation Z, beam direction (mm) - bool isWorld = false; ///< true for the mother/world volume (the cavern) + const char* name; ///< registry key / CLI name, e.g. "Calorimeter" + const char* node; ///< GeoNameTag, e.g. "/SHiP/calorimeter" + int id; ///< GeoIdentifierTag + double x_mm; ///< placement translation X (mm) + double y_mm; ///< placement translation Y (mm) + double z_mm; ///< placement translation Z, beam direction (mm) + bool isWorld = false; ///< true for the mother/world volume (the cavern) }; /// A registered subsystem: its descriptor plus how to build it (local frame). @@ -83,11 +83,10 @@ std::vector subsystemNames(); * references it, the subsystem archive must be linked with --whole-archive * (or an equivalent) so the initialiser is not dropped by the linker. */ -#define REGISTER_SUBSYSTEM(FACTORY) \ - namespace { \ - const bool FACTORY##_registered = ::SHiPGeometry::registerSubsystem( \ - FACTORY::descriptor(), \ - [](::SHiPGeometry::SHiPMaterials& materials) -> ::GeoVPhysVol* { \ - return FACTORY(materials).build(); \ - }); \ +#define REGISTER_SUBSYSTEM(FACTORY) \ + namespace { \ + const bool FACTORY##_registered = ::SHiPGeometry::registerSubsystem( \ + FACTORY::descriptor(), [](::SHiPGeometry::SHiPMaterials& materials) -> ::GeoVPhysVol* { \ + return FACTORY(materials).build(); \ + }); \ } diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d6ef4c7..63ed9b4 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -10,19 +10,12 @@ target_include_directories( $ $ ) - target_link_libraries( SHiPGeometry PUBLIC GeoModelCore::GeoModelKernel GeoModelIO::GeoModelRead GeoModelIO::GeoModelWrite - # Force-link the subsystem archives: each registers itself at - # static-init via REGISTER_SUBSYSTEM, and nothing here references - # their symbols, so without --whole-archive the linker would drop - # them and the registry would be empty. (GNU/Clang syntax; for - # CMake >= 3.24 prefer $.) - "-Wl,--whole-archive" Cavern Target MuonShield @@ -33,5 +26,11 @@ target_link_libraries( Magnet TimingDetector Calorimeter - "-Wl,--no-whole-archive" ) + +# The subsystems are referenced only through their REGISTER_SUBSYSTEM static +# initialisers, so the toolchain's default --as-needed drops them from the +# DT_NEEDED of every executable that links SHiPGeometry, and their +# registrations never run. INTERFACE puts --no-as-needed on the *consumer's* +# link line, which is where the entries were being dropped. +target_link_options(SHiPGeometry INTERFACE "-Wl,--no-as-needed") diff --git a/src/SHiPGeometry.cpp b/src/SHiPGeometry.cpp index fdff79d..9d21023 100644 --- a/src/SHiPGeometry.cpp +++ b/src/SHiPGeometry.cpp @@ -63,13 +63,15 @@ GeoPhysVol* assembleGeometry(const std::vector& only) { std::vector placed; for (auto& entry : reg) { const SubsystemInfo& info = entry.second; - if (info.desc.isWorld) continue; + if (info.desc.isWorld) + continue; if (only.empty() || std::find(only.begin(), only.end(), entry.first) != only.end()) { placed.push_back(&info); } } std::sort(placed.begin(), placed.end(), [](const SubsystemInfo* a, const SubsystemInfo* b) { - if (a->desc.z_mm != b->desc.z_mm) return a->desc.z_mm < b->desc.z_mm; + if (a->desc.z_mm != b->desc.z_mm) + return a->desc.z_mm < b->desc.z_mm; return a->desc.id < b->desc.id; }); @@ -91,7 +93,8 @@ GeoVPhysVol* buildSubsystem(const std::string& name) { std::vector subsystemNames() { std::vector names; - for (auto& entry : registry()) names.push_back(entry.first); + for (auto& entry : registry()) + names.push_back(entry.first); return names; } diff --git a/subsystems/Calorimeter/include/Calorimeter/CalorimeterFactory.h b/subsystems/Calorimeter/include/Calorimeter/CalorimeterFactory.h index 41427e0..662cdf2 100644 --- a/subsystems/Calorimeter/include/Calorimeter/CalorimeterFactory.h +++ b/subsystems/Calorimeter/include/Calorimeter/CalorimeterFactory.h @@ -3,9 +3,10 @@ #pragma once -#include #include "SHiPGeometry/SubsystemRegistry.h" +#include + #include class GeoPhysVol; @@ -33,7 +34,6 @@ class CalorimeterFactory { explicit CalorimeterFactory(SHiPMaterials& materials, std::string configPath = "calo.toml"); ~CalorimeterFactory() = default; - /// This subsystem's self-description (name, node, id, placement). static SubsystemDescriptor descriptor() { return {"Calorimeter", "/SHiP/calorimeter", 8, 0.0, 0.0, 98320.0, false}; diff --git a/subsystems/Calorimeter/src/CalorimeterFactory.cpp b/subsystems/Calorimeter/src/CalorimeterFactory.cpp index 0e26d4b..374c60d 100644 --- a/subsystems/Calorimeter/src/CalorimeterFactory.cpp +++ b/subsystems/Calorimeter/src/CalorimeterFactory.cpp @@ -2,13 +2,13 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "Calorimeter/CalorimeterFactory.h" -#include "SHiPGeometry/SubsystemRegistry.h" #include "Calorimeter/CaloBarLayer.h" #include "Calorimeter/CaloFibreHPLayer.h" #include "Calorimeter/CalorimeterConfig.h" #include "SHiPGeometry/ConfigPath.h" #include "SHiPGeometry/SHiPMaterials.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include #include diff --git a/subsystems/Cavern/include/Cavern/CavernFactory.h b/subsystems/Cavern/include/Cavern/CavernFactory.h index 1ff80f6..c470aaa 100644 --- a/subsystems/Cavern/include/Cavern/CavernFactory.h +++ b/subsystems/Cavern/include/Cavern/CavernFactory.h @@ -21,7 +21,6 @@ class CavernFactory { explicit CavernFactory(SHiPMaterials& materials); ~CavernFactory() = default; - /// This subsystem's self-description (name, node, id, placement). static SubsystemDescriptor descriptor() { return {"Cavern", "/SHiP/cavern", 0, 0.0, 0.0, 0.0, true}; diff --git a/subsystems/Cavern/src/CavernFactory.cpp b/subsystems/Cavern/src/CavernFactory.cpp index 912fd09..2167c6d 100644 --- a/subsystems/Cavern/src/CavernFactory.cpp +++ b/subsystems/Cavern/src/CavernFactory.cpp @@ -2,9 +2,9 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "Cavern/CavernFactory.h" -#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include #include diff --git a/subsystems/DecayVolume/include/DecayVolume/DecayVolumeFactory.h b/subsystems/DecayVolume/include/DecayVolume/DecayVolumeFactory.h index 02c161c..7fd9927 100644 --- a/subsystems/DecayVolume/include/DecayVolume/DecayVolumeFactory.h +++ b/subsystems/DecayVolume/include/DecayVolume/DecayVolumeFactory.h @@ -31,7 +31,6 @@ class DecayVolumeFactory { explicit DecayVolumeFactory(SHiPMaterials& materials, std::string configPath = "sbt.toml"); ~DecayVolumeFactory() = default; - /// This subsystem's self-description (name, node, id, placement). static SubsystemDescriptor descriptor() { return {"DecayVolume", "/SHiP/decay_volume", 4, 0.0, 0.0, 58120.0, false}; diff --git a/subsystems/DecayVolume/src/DecayVolumeFactory.cpp b/subsystems/DecayVolume/src/DecayVolumeFactory.cpp index be202b9..b80cd5e 100644 --- a/subsystems/DecayVolume/src/DecayVolumeFactory.cpp +++ b/subsystems/DecayVolume/src/DecayVolumeFactory.cpp @@ -2,13 +2,13 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "DecayVolume/DecayVolumeFactory.h" -#include "SHiPGeometry/SubsystemRegistry.h" #include "DecayVolume/SBTConfig.h" #include "DecayVolume/SBTSensorBuilder.h" #include "DecayVolume/SBTStructureBuilder.h" #include "SHiPGeometry/ConfigPath.h" #include "SHiPGeometry/SHiPMaterials.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include #include diff --git a/subsystems/Magnet/include/Magnet/MagnetFactory.h b/subsystems/Magnet/include/Magnet/MagnetFactory.h index 46415f0..52cb4d0 100644 --- a/subsystems/Magnet/include/Magnet/MagnetFactory.h +++ b/subsystems/Magnet/include/Magnet/MagnetFactory.h @@ -3,9 +3,10 @@ #pragma once -#include #include "SHiPGeometry/SubsystemRegistry.h" +#include + #include class GeoPhysVol; @@ -27,7 +28,6 @@ class MagnetFactory { explicit MagnetFactory(SHiPMaterials& materials); ~MagnetFactory() = default; - /// This subsystem's self-description (name, node, id, placement). static SubsystemDescriptor descriptor() { return {"Magnet", "/SHiP/magnet", 6, 0.0, 0.0, 89570.0, false}; diff --git a/subsystems/Magnet/src/MagnetFactory.cpp b/subsystems/Magnet/src/MagnetFactory.cpp index f1fa35e..35a15e9 100644 --- a/subsystems/Magnet/src/MagnetFactory.cpp +++ b/subsystems/Magnet/src/MagnetFactory.cpp @@ -2,9 +2,9 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "Magnet/MagnetFactory.h" -#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include #include diff --git a/subsystems/MuonShield/include/MuonShield/MuonShieldFactory.h b/subsystems/MuonShield/include/MuonShield/MuonShieldFactory.h index 25246e1..fee97f4 100644 --- a/subsystems/MuonShield/include/MuonShield/MuonShieldFactory.h +++ b/subsystems/MuonShield/include/MuonShield/MuonShieldFactory.h @@ -3,9 +3,10 @@ #pragma once -#include #include "SHiPGeometry/SubsystemRegistry.h" +#include + class GeoPhysVol; namespace SHiPGeometry { @@ -32,7 +33,6 @@ class MuonShieldFactory { static SubsystemDescriptor descriptor() { return {"MuonShield", "/SHiP/muon_shield", 2, 0.0, 0.0, 16763.3, false}; } - GeoPhysVol* build(); private: SHiPMaterials& m_materials; diff --git a/subsystems/MuonShield/src/MuonShieldFactory.cpp b/subsystems/MuonShield/src/MuonShieldFactory.cpp index 8658771..838851d 100644 --- a/subsystems/MuonShield/src/MuonShieldFactory.cpp +++ b/subsystems/MuonShield/src/MuonShieldFactory.cpp @@ -2,9 +2,9 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "MuonShield/MuonShieldFactory.h" -#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include #include diff --git a/subsystems/NeutrinoDetector/include/NeutrinoDetector/NeutrinoDetectorFactory.h b/subsystems/NeutrinoDetector/include/NeutrinoDetector/NeutrinoDetectorFactory.h index ec6e2ca..4ed9b50 100644 --- a/subsystems/NeutrinoDetector/include/NeutrinoDetector/NeutrinoDetectorFactory.h +++ b/subsystems/NeutrinoDetector/include/NeutrinoDetector/NeutrinoDetectorFactory.h @@ -51,7 +51,6 @@ class NeutrinoDetectorFactory { /// Defaulted destructor. ~NeutrinoDetectorFactory() = default; - /// This subsystem's self-description (name, node, id, placement). static SubsystemDescriptor descriptor() { return {"NeutrinoDetector", "/SHiP/neutrino_detector", 9, 0.0, 0.0, 28950.0, false}; diff --git a/subsystems/NeutrinoDetector/src/NeutrinoDetectorFactory.cpp b/subsystems/NeutrinoDetector/src/NeutrinoDetectorFactory.cpp index aed2df3..78d82fb 100644 --- a/subsystems/NeutrinoDetector/src/NeutrinoDetectorFactory.cpp +++ b/subsystems/NeutrinoDetector/src/NeutrinoDetectorFactory.cpp @@ -2,9 +2,9 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "NeutrinoDetector/NeutrinoDetectorFactory.h" -#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include #include diff --git a/subsystems/Target/include/Target/TargetFactory.h b/subsystems/Target/include/Target/TargetFactory.h index 576561f..925179d 100644 --- a/subsystems/Target/include/Target/TargetFactory.h +++ b/subsystems/Target/include/Target/TargetFactory.h @@ -31,7 +31,6 @@ class TargetFactory { explicit TargetFactory(SHiPMaterials& materials); ~TargetFactory() = default; - /// This subsystem's self-description (name, node, id, placement). static SubsystemDescriptor descriptor() { return {"Target", "/SHiP/target", 1, 0.0, -144.5, 432.5, false}; diff --git a/subsystems/Target/src/TargetFactory.cpp b/subsystems/Target/src/TargetFactory.cpp index e8d816e..4acb28e 100644 --- a/subsystems/Target/src/TargetFactory.cpp +++ b/subsystems/Target/src/TargetFactory.cpp @@ -2,9 +2,9 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "Target/TargetFactory.h" -#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include #include diff --git a/subsystems/TimingDetector/include/TimingDetector/TimingDetectorFactory.h b/subsystems/TimingDetector/include/TimingDetector/TimingDetectorFactory.h index 583ab31..d252fd2 100644 --- a/subsystems/TimingDetector/include/TimingDetector/TimingDetectorFactory.h +++ b/subsystems/TimingDetector/include/TimingDetector/TimingDetectorFactory.h @@ -3,9 +3,10 @@ #pragma once -#include #include "SHiPGeometry/SubsystemRegistry.h" +#include + class GeoPhysVol; namespace SHiPGeometry { @@ -27,7 +28,6 @@ class TimingDetectorFactory { explicit TimingDetectorFactory(SHiPMaterials& materials); ~TimingDetectorFactory() = default; - /// This subsystem's self-description (name, node, id, placement). static SubsystemDescriptor descriptor() { return {"TimingDetector", "/SHiP/timing_detector", 7, 0.0, 0.0, 95902.0, false}; diff --git a/subsystems/TimingDetector/src/TimingDetectorFactory.cpp b/subsystems/TimingDetector/src/TimingDetectorFactory.cpp index 37b2889..91224ad 100644 --- a/subsystems/TimingDetector/src/TimingDetectorFactory.cpp +++ b/subsystems/TimingDetector/src/TimingDetectorFactory.cpp @@ -2,9 +2,9 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "TimingDetector/TimingDetectorFactory.h" -#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "TimingDetector/SHiPTimingDetInterface.h" #include diff --git a/subsystems/Trackers/include/Trackers/TrackersFactory.h b/subsystems/Trackers/include/Trackers/TrackersFactory.h index 1579618..0747050 100644 --- a/subsystems/Trackers/include/Trackers/TrackersFactory.h +++ b/subsystems/Trackers/include/Trackers/TrackersFactory.h @@ -3,9 +3,10 @@ #pragma once -#include #include "SHiPGeometry/SubsystemRegistry.h" +#include + #include #include @@ -41,7 +42,6 @@ class TrackersFactory { explicit TrackersFactory(SHiPMaterials& materials); ~TrackersFactory() = default; - /// This subsystem's self-description (name, node, id, placement). static SubsystemDescriptor descriptor() { return {"Trackers", "/SHiP/trackers", 5, 0.0, 0.0, 89570.0, false}; diff --git a/subsystems/Trackers/src/TrackersFactory.cpp b/subsystems/Trackers/src/TrackersFactory.cpp index f494756..8134cd9 100644 --- a/subsystems/Trackers/src/TrackersFactory.cpp +++ b/subsystems/Trackers/src/TrackersFactory.cpp @@ -2,9 +2,9 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "Trackers/TrackersFactory.h" -#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include #include diff --git a/subsystems/UpstreamTagger/include/UpstreamTagger/UpstreamTaggerFactory.h b/subsystems/UpstreamTagger/include/UpstreamTagger/UpstreamTaggerFactory.h index fb8c253..854800d 100644 --- a/subsystems/UpstreamTagger/include/UpstreamTagger/UpstreamTaggerFactory.h +++ b/subsystems/UpstreamTagger/include/UpstreamTagger/UpstreamTaggerFactory.h @@ -54,7 +54,6 @@ class UpstreamTaggerFactory { explicit UpstreamTaggerFactory(SHiPMaterials& materials); ~UpstreamTaggerFactory() = default; - /// This subsystem's self-description (name, node, id, placement). static SubsystemDescriptor descriptor() { return {"UpstreamTagger", "/SHiP/upstream_tagger", 3, 0.0, 0.0, 32720.0, false}; diff --git a/subsystems/UpstreamTagger/src/UpstreamTaggerFactory.cpp b/subsystems/UpstreamTagger/src/UpstreamTaggerFactory.cpp index daf1f7f..6bc0726 100644 --- a/subsystems/UpstreamTagger/src/UpstreamTaggerFactory.cpp +++ b/subsystems/UpstreamTagger/src/UpstreamTaggerFactory.cpp @@ -2,9 +2,9 @@ // Copyright (C) CERN for the benefit of the SHiP Collaboration #include "UpstreamTagger/UpstreamTaggerFactory.h" -#include "SHiPGeometry/SubsystemRegistry.h" #include "SHiPGeometry/SHiPMaterials.h" +#include "SHiPGeometry/SubsystemRegistry.h" #include "UpstreamTagger/SHiPUBTManager.h" #include From 6bddf29f540c48e82cbcb565aac507573d9ed8c6 Mon Sep 17 00:00:00 2001 From: Matei Climescu Date: Thu, 2 Jul 2026 00:34:50 +0200 Subject: [PATCH 3/4] fix: updated CMakefile --- src/CMakeLists.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 63ed9b4..02d8dfb 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,9 +1,7 @@ # SPDX-License-Identifier: LGPL-3.0-or-later # Copyright (C) CERN for the benefit of the SHiP Collaboration -# Create SHiPGeometry library add_library(SHiPGeometry SHiPMaterials.cpp SHiPGeometry.cpp) - target_include_directories( SHiPGeometry PUBLIC From 135f7390382835868db1c5655ac7e1fa628314bf Mon Sep 17 00:00:00 2001 From: Matei Climescu Date: Sun, 12 Jul 2026 01:29:57 +0200 Subject: [PATCH 4/4] fix(registry): reject duplicate subsystem names and guard the link option registerSubsystem inspected nothing from emplace(), so a second subsystem declaring an already-registered descriptor name was silently discarded and registration still reported success. Check the insertion result and abort with a diagnostic naming the offending subsystem; registration runs at static-init, so a throw would reach no handler. Add , and so the header is self-contained, and correct the REGISTER_SUBSYSTEM note, which still described the subsystems as static archives needing --whole-archive. Restrict -Wl,--no-as-needed to non-Apple UNIX, and add NeutrinoDetector to the README structure tree. --- include/SHiPGeometry/SubsystemRegistry.h | 26 +++++++++++++++++++----- src/CMakeLists.txt | 4 ++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/include/SHiPGeometry/SubsystemRegistry.h b/include/SHiPGeometry/SubsystemRegistry.h index 6fa2b7d..29bec6d 100644 --- a/include/SHiPGeometry/SubsystemRegistry.h +++ b/include/SHiPGeometry/SubsystemRegistry.h @@ -5,9 +5,12 @@ #include // complete GeoPhysVol/GeoVPhysVol for the macro's upcast +#include +#include #include #include #include +#include #include namespace SHiPGeometry { @@ -50,10 +53,21 @@ inline std::map& registry() { return instance; } -/// Add a subsystem to the registry. Returns true (usable as a static initialiser). +/// Add a subsystem to the registry. Returns true (usable as a static +/// initialiser). A duplicate name is a programming error (two subsystems +/// declaring the same descriptor name): it is reported and aborts, rather +/// than being silently dropped by emplace(). Runs at static-init, so this +/// diagnoses to stderr and aborts instead of throwing. inline bool registerSubsystem(const SubsystemDescriptor& desc, std::function build) { - registry().emplace(desc.name, SubsystemInfo{desc, std::move(build)}); + const auto result = registry().emplace(desc.name, SubsystemInfo{desc, std::move(build)}); + if (!result.second) { + std::fprintf(stderr, + "SHiPGeometry: duplicate subsystem name '%s' registered; " + "each subsystem's descriptor() must return a unique name.\n", + desc.name); + std::abort(); + } return true; } @@ -79,9 +93,11 @@ std::vector subsystemNames(); * The factory must expose `static SubsystemDescriptor descriptor()` and be * constructible from `SHiPMaterials&` with a `build()` returning a volume. * - * NOTE: because this registration lives in a static library and nothing else - * references it, the subsystem archive must be linked with --whole-archive - * (or an equivalent) so the initialiser is not dropped by the linker. + * NOTE: nothing references this registration, so the subsystem library would + * otherwise be dropped from a consumer's DT_NEEDED by the toolchain's default + * --as-needed and the initialiser would never run. src/CMakeLists.txt applies + * -Wl,--no-as-needed as an INTERFACE link option on SHiPGeometry so the flag + * lands on each executable's link line. Do not remove it. */ #define REGISTER_SUBSYSTEM(FACTORY) \ namespace { \ diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 02d8dfb..a45aeab 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -32,3 +32,7 @@ target_link_libraries( # registrations never run. INTERFACE puts --no-as-needed on the *consumer's* # link line, which is where the entries were being dropped. target_link_options(SHiPGeometry INTERFACE "-Wl,--no-as-needed") + +if(UNIX AND NOT APPLE) + target_link_options(SHiPGeometry INTERFACE "-Wl,--no-as-needed") +endif()