Skip to content

style: make every dimensional literal carry an explicit unit #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
olantwin merged 2 commits into from
Jul 10, 2026
Merged

style: make every dimensional literal carry an explicit unit #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36d7997
style: make every dimensional literal carry an explicit unit
olantwin Jul 9, 2026
9cd9565
docs: correct Magnet container-dimensions comment
olantwin Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ An [automatic class reference](https://shipsoft.github.io/Geometry/) is built us
- **Units**: mm (GeoModel native), angles in radians
- **Beam axis height**: 1.7 m above floor

### Unit convention

Every dimensional literal (length or angle) in the code carries an explicit
`GeoModelKernelUnits` unit, e.g. `3000.0 * GeoModelKernelUnits::mm` or
`90.0 * deg` — either at its definition or, for config-driven values with an
`_mm` suffix, at the point of conversion into GeoModel units. GeoModel's
native length unit is mm (`GeoModelKernelUnits::mm == 1.0`), so the
annotations are numerically free; they exist to make the unit of every
quantity explicit at the point where it is written.

## Implementation Status

| Subsystem | Status | Description |
Expand Down
23 changes: 14 additions & 9 deletions src/SHiPGeometry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ SHiPGeometryBuilder::SHiPGeometryBuilder() = default;
SHiPGeometryBuilder::~SHiPGeometryBuilder() = default;

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;

// Create central materials manager
SHiPMaterials materials;

Expand All @@ -41,14 +46,14 @@ GeoPhysVol* SHiPGeometryBuilder::build() {

// Position target in world (from GDML: x=0, y=-14.45cm, z=43.25cm)
// Note: These are relative to the cave origin
constexpr double cm = GeoModelKernelUnits::cm;
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));
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.
Expand All @@ -57,52 +62,52 @@ GeoPhysVol* SHiPGeometryBuilder::build() {
NeutrinoDetectorFactory neutrinoDetectorFactory(materials);
GeoPhysVol* neutrinoDetector = neutrinoDetectorFactory.build();
placeChild(world, neutrinoDetector, "/SHiP/neutrino_detector", 9,
GeoTrf::Translate3D(0.0, 0.0, 28.95 * 1000.0));
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 * 1000.0));
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 * 1000.0));
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 * 1000.0;
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 * 1000.0));
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 * 1000.0));
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 * 1000.0));
GeoTrf::Translate3D(0.0, 0.0, 98.32 * m));

return world;
}
Expand Down
10 changes: 6 additions & 4 deletions subsystems/Calorimeter/include/Calorimeter/CalorimeterFactory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

#pragma once

#include <GeoModelKernel/Units.h>

#include <string>

class GeoPhysVol;
Expand Down Expand Up @@ -49,12 +51,12 @@ class CalorimeterFactory {
void buildStack(GeoPhysVol* container, const CalorimeterConfig& cfg, int moduleX, int moduleY,
double offsetX, double offsetY) const;

// ── Fixed container dimensions (mm) ─────────────────────────────────
// ── Fixed container dimensions ──────────────────────────────────────
// These match the SHiP subsystem envelope from subsystem_envelopes.csv
// and must not change — tests and the consistency check depend on them.
static constexpr double s_containerHalfX = 3000.0; // 3.00 m
static constexpr double s_containerHalfY = 3500.0; // 3.50 m
static constexpr double s_containerHalfZ = 1450.0; // 1.45 m
static constexpr double s_containerHalfX = 3000.0 * GeoModelKernelUnits::mm; // 3.00 m
static constexpr double s_containerHalfY = 3500.0 * GeoModelKernelUnits::mm; // 3.50 m
static constexpr double s_containerHalfZ = 1450.0 * GeoModelKernelUnits::mm; // 1.45 m
};

} // namespace SHiPGeometry
50 changes: 28 additions & 22 deletions subsystems/Magnet/include/Magnet/MagnetFactory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

#pragma once

#include <GeoModelKernel/Units.h>

#include <string>

class GeoPhysVol;
Expand Down Expand Up @@ -37,33 +39,37 @@ class MagnetFactory {
GeoPhysVol* createCoil(const std::string& name);
GeoPhysVol* createVerticalConnector(const std::string& name);

// Yoke dimensions from GDML (half-sizes in mm)
// Unit shorthand (GeoModel's native length unit is mm)
static constexpr double mm = GeoModelKernelUnits::mm;

// Yoke dimensions from GDML (half-sizes)
// GDML: outer 600×860×280 cm, inner 440×700×282 cm
static constexpr double s_yokeOuterHalfX = 3000.0;
static constexpr double s_yokeOuterHalfY = 4300.0;
static constexpr double s_yokeOuterHalfZ = 1400.0;
static constexpr double s_yokeInnerHalfX = 2200.0;
static constexpr double s_yokeInnerHalfY = 3500.0;
static constexpr double s_yokeInnerHalfZ = 1410.0;
static constexpr double s_yokeOuterHalfX = 3000.0 * mm;
static constexpr double s_yokeOuterHalfY = 4300.0 * mm;
static constexpr double s_yokeOuterHalfZ = 1400.0 * mm;
static constexpr double s_yokeInnerHalfX = 2200.0 * mm;
static constexpr double s_yokeInnerHalfY = 3500.0 * mm;
static constexpr double s_yokeInnerHalfZ = 1410.0 * mm;

// Coil dimensions (simplified as boxes)
static constexpr double s_coilHalfX = 800.0;
static constexpr double s_coilHalfY = 400.0;
static constexpr double s_coilHalfZ = 1660.0;
static constexpr double s_coilXOffset = 2200.0;
static constexpr double s_coilYOffset = 3250.0;
static constexpr double s_coilHalfX = 800.0 * mm;
static constexpr double s_coilHalfY = 400.0 * mm;
static constexpr double s_coilHalfZ = 1660.0 * mm;
static constexpr double s_coilXOffset = 2200.0 * mm;
static constexpr double s_coilYOffset = 3250.0 * mm;

// Vertical connector dimensions from GDML: 80×650×25 cm
static constexpr double s_connectorHalfX = 400.0;
static constexpr double s_connectorHalfY = 3250.0;
static constexpr double s_connectorHalfZ = 125.0;
static constexpr double s_connectorXOffset = 2600.0;
static constexpr double s_connectorZOffset = 1525.0; // From GDML positions

// Container (same as yoke outer for simplicity)
static constexpr double s_containerHalfX = 3250.0;
static constexpr double s_containerHalfY = 4300.0;
static constexpr double s_containerHalfZ = 2500.0;
static constexpr double s_connectorHalfX = 400.0 * mm;
static constexpr double s_connectorHalfY = 3250.0 * mm;
static constexpr double s_connectorHalfZ = 125.0 * mm;
static constexpr double s_connectorXOffset = 2600.0 * mm;
static constexpr double s_connectorZOffset = 1525.0 * mm; // From GDML positions

// Container sized to enclose the yoke, coils, and connectors: HalfX and
// HalfZ exceed the yoke outer dimensions, while HalfY matches it.
static constexpr double s_containerHalfX = 3250.0 * mm;
static constexpr double s_containerHalfY = 4300.0 * mm;
static constexpr double s_containerHalfZ = 2500.0 * mm;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
};

} // namespace SHiPGeometry
19 changes: 12 additions & 7 deletions subsystems/MuonShield/include/MuonShield/MuonShieldFactory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

#pragma once

#include <GeoModelKernel/Units.h>

class GeoPhysVol;

namespace SHiPGeometry {
Expand All @@ -28,15 +30,18 @@ class MuonShieldFactory {
private:
SHiPMaterials& m_materials;

// Unit shorthand (GeoModel's native length unit is mm)
static constexpr double mm = GeoModelKernelUnits::mm;

struct PieceData {
double halfX, halfY, halfZ; // bounding-box half-sizes (mm)
double centX, centY; // centre offset in station XY frame (mm)
double halfX, halfY, halfZ; // bounding-box half-sizes
double centX, centY; // centre offset in station XY frame
const char* name; // piece name suffix
};

struct StationData {
const char* name; // station name
double stationZ; // z in MuonShieldArea frame (mm)
double stationZ; // z in MuonShieldArea frame
double containerHalfX;
double containerHalfY;
double containerHalfZ;
Expand All @@ -48,10 +53,10 @@ class MuonShieldFactory {

GeoPhysVol* buildStation(const StationData& station);

// MuonShieldArea container dimensions (mm)
static constexpr double s_areaHalfX = 1810.0;
static constexpr double s_areaHalfY = 1700.0;
static constexpr double s_areaHalfZ = 14724.0;
// MuonShieldArea container dimensions
static constexpr double s_areaHalfX = 1810.0 * mm;
static constexpr double s_areaHalfY = 1700.0 * mm;
static constexpr double s_areaHalfZ = 14724.0 * mm;
};

} // namespace SHiPGeometry
Loading
Loading