Skip to content

feat(geom): add Run1B production geometry and virtual detector ids - #1923

Open
oksuzian wants to merge 1 commit into
mainfrom
run1b-v40-geometry
Open

feat(geom): add Run1B production geometry and virtual detector ids#1923
oksuzian wants to merge 1 commit into
mainfrom
run1b-v40-geometry

Conversation

@oksuzian

@oksuzian oksuzian commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Ports the Run1B production geometry to main.

geom_common.txt is deliberately not changed — main keeps
geom_run1_a_stickman.txt as its default. That was the review comment that
blocked #1849, and it is addressed by construction here.

Why

Production main's Tests/Run1BReco.fcl and the prodtools Run1B digi/reco
templates already reference Offline/Mu2eG4/geom/geom_run1_b_v40.txt, which
today exists only on the Run1B branch. This repairs that dangling cross-repo
reference.

Only v40 is ported

Run1B has two stopping targets: the thin 37-foil target shared with Run1A, and
a disk at the face of TS5. geom_run1_b_v40.txt models this correctly — it
inherits geom_run1_a.txt untouched, so the 37 foils and their support
structure survive, and adds a two-piece aluminium assembly at the TS5 face (the
TSdA as a 600 mm plate with a 135 mm aperture at z = 4195, and the pion degrader
repurposed as a 150 mm mobile target at z = 4235). ds_on_v40 is v40 with the
mobile target rotated out of the beam.

The earlier v01–v06 line instead overrode stoppingTarget.radii to a single
600 mm disk, replacing the foil target rather than adding to it. v40
superseded that line and is what the Run1Ban and Run1Bap campaigns run, so only
v40 is ported here.

Everything v40 needs already exists on main — no constructTSdA.cc,
constructStoppingTarget.cc or Mu2eWorld.cc changes are required. Confirmed
from the dump: v40 yields TSdA4 at rmax 600 / rmin 135 / z 17.5 in
StoppingTarget_Al (nominal gives 525 / 235 / 50.8), and its stopping-target
foil count is identical to geom_run1_a.txt.

Tracker FEB virtual detectors, off by default

19 identifiers (117–135) and the code that builds them: annular virtual
detectors on the upstream face of every second tracker plane's front-end board,
plus one behind the last plane.

These are gated on a new key, hasTrackerFEBVirtualDetectors, defaulting
false
, which no geometry file sets. This deliberately differs from the Run1B
branch, which gates them on TrackerHasBrassRings — a key set true in
tracker_v4/v5/v6/v7.txt, i.e. every current geometry, so on the branch these
detectors are built in nominal running. Re-gating keeps the capability available
without changing anyone's simulation.

The registration block is the only place the key is read; placement follows via
the existing vdg->exist(vdId) guard and is additionally fenced behind
vdg->exist(Tracker_FEB_0_SurfIn), so the disabled path constructs nothing and
performs no geometry lookups. Diagnostic output is behind verbosityLevel > 0,
matching the surrounding code.

Enum safety

The identifiers are appended before lastEnum, so no existing enumerator
changes position or value and existing art files stay readable.
VIRTUALDETECTORID_NAMES gains 19 matching strings in the same order; the
BOOST_STATIC_ASSERT at VirtualDetectorId.cc:65 is satisfied at 136 == 136,
verified pairwise across all 136 entries. A survey of lastEnum consumers found
no fixed-size array dimensioned on a literal, no persisted format with a baked-in
width, and no switch assuming a maximum id.

Per review, the branch's EMC_Source, EMC_Source2 and EMC_0_Front
identifiers are not included — the first two served a study calorimeter
between Coll5 and the tracker rather than production hardware, and EMC_0_Front
duplicates the existing EMC_0_FrontIn/EMC_0_FrontOut that already back
isDisk0().

Verification

Normalized gdml dumps (Geant4 pointer suffixes stripped) of both nominal
geometries are byte-identical to main:

geometry volumes md5, unchanged
geom_common.txt 13767 85b9f421823ab803c4250e95214ff8f1
geom_run1_a.txt 14355 1d0cbf23ebbae35a2c14660cc535f90b

The harness was validated before being trusted: two dumps of the same unchanged
geometry are byte-identical after normalization, while the raw pre-normalization
dumps differ by 125,118 lines from pointer addresses alone.

The FEB detectors were also exercised in the enabled state — a scratch geometry
setting hasTrackerFEBVirtualDetectors = true on top of v40 produces exactly 19
volumes at copy numbers 117–135 with no Geant4 overlaps, while a plain v40 dump
produces none and is byte-identical to v40 before this change.

@FNALbuild

Copy link
Copy Markdown
Collaborator

Hi @oksuzian,
You have proposed changes to files in these packages:

  • Mu2eG4
  • DataProducts

which require these tests: build.

@Mu2e/write, @Mu2e/fnalbuild-users have access to CI actions on main.

⌛ The following tests have been triggered for b4620c6: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at b4620c6.

Test Result Details
test with Command did not list any other PRs to include
merge Merged b4620c6 at 1891b32
build (prof) Log file. Build time: 04 min 27 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
trigger Log file.
check_cmake Log file.
FIXME, TODO ➡️ TODO (0) FIXME (1) in 1 files
clang-tidy 0 errors 0 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at b4620c6 after being merged into the base branch at 1891b32.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@sdifalco

sdifalco commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator
immagine Virtual detectors EMCSource and EMCSource2 were introduced to study rates and doses in a small calorimeter placed between Coll5 and the tracker. I think they should not be inserted.

@sdifalco

sdifalco commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator
immagine Also VirtualDetector_EMC_0_Front is obsolete. immagine The existing virtual detectors surrounding Disk 0 should be enough for radiation studies

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to delete EMC_Source, EMC_Source2 and EMC_0_Front

@michaelmackenzie michaelmackenzie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me

@oksuzian

oksuzian commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

@FNALbuild run build test

@FNALbuild

Copy link
Copy Markdown
Collaborator

⌛ The following tests have been triggered for 615cf5c: build (Build queue - API unavailable)

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 615cf5c.

Test Result Details
test with Command did not list any other PRs to include
merge Merged 615cf5c at 1891b32
build (prof) Log file. Build time: 08 min 49 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
trigger Log file.
check_cmake Log file.
FIXME, TODO ➡️ TODO (0) FIXME (1) in 1 files
clang-tidy 0 errors 0 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 615cf5c after being merged into the base branch at 1891b32.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

Adds geom_run1_b_v40.txt and its DS-on counterpart, a gdml dump driver
for v40, and 19 tracker FEB virtual detector identifiers (117-135),
together with the code that registers and places them.

The FEB virtual detectors are ported from the Run1B branch behind a new
configuration key, hasTrackerFEBVirtualDetectors, which defaults to
false and is set by no geometry file in the repository, so no existing
geometry is affected. This deliberately differs from the Run1B branch,
which gates the same code on TrackerHasBrassRings -- a key about brass
rings in the tracker, not about virtual detectors, and one set true in
tracker_v4 through tracker_v7, i.e. in every current tracker geometry.
On the branch these detectors are therefore built in nominal running;
here they are strictly opt-in. geom_run1_b_v40.txt does not set the key
either: the capability ships, opting in is a separate one-line decision.
The unguarded cout diagnostics that came with the branch code are folded
into the verbosityLevel > 0 blocks used by the surrounding code.

The placement loop in constructVirtualDetectors.cc is entered only when
VirtualDetectorMaker has registered the identifiers, so with the key off
it does no work at all. VirtualDetectorId::isFEBTracker() is a
three-line enum-range predicate matching the existing
isTrackerFront()/isTrackerBack() accessors in the same header.
geom_common.txt is untouched, so nominal running is unaffected --
verified by normalized gdml dumps of geom_common.txt and geom_run1_a.txt
being byte-identical to main (13767 and 14355 volumes), and by a plain
v40 dump containing zero FEB volumes. With the key forced true on top of
v40 in a scratch geometry, exactly 19 FEB volumes appear (copy numbers
117-135) and Geant4 reports no overlaps.

The new identifiers are appended before lastEnum, so no existing
enumerator changes position or value. VIRTUALDETECTORID_NAMES gains 19
matching strings in the same order, keeping the BOOST_STATIC_ASSERT in
VirtualDetectorId.cc:65 satisfied at 136 == 136.

Repairs a dangling cross-repo reference: Production main's
Tests/Run1BReco.fcl already points at geom_run1_b_v40.txt, which exists
only on the Run1B branch today.

Per review, the EMC_Source, EMC_Source2 and EMC_0_Front identifiers
carried on the Run1B branch are not included. The first two served a
study calorimeter between Coll5 and the tracker rather than production
hardware, and EMC_0_Front duplicates the existing EMC_0_FrontIn and
EMC_0_FrontOut detectors that already back isDisk0(). None of the three
is referenced by any code, fcl or geometry file. No EMC code is ported
from the branch either.

Four commented-out blocks are dropped from geom_run1_b_v40.txt. They
recorded an abandoned attempt to extend DS2Vacuum over the tracker, and
the first of them -- tracker.inDS2Vacuum -- is read by no code on main,
so uncommenting it would have silently done nothing. The v40 gdml dump
is byte-identical with and without them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@oksuzian
oksuzian force-pushed the run1b-v40-geometry branch from 2ce8202 to 7523dd0 Compare August 9, 2026 22:18
@oksuzian

Copy link
Copy Markdown
Collaborator Author

@FNALbuild run build test

@oksuzian

Copy link
Copy Markdown
Collaborator Author

@sdifalco Thanks — agreed on both counts, and all three identifiers are removed as of 615cf5c.

Checking before cutting them, your case was if anything stronger than stated: EMC_0_Front would have sat alongside EMC_0_FrontIn and EMC_0_FrontOut (49, 50), which already exist and already back isDisk0(). And none of the three was referenced by any code, fcl or geometry file in this PR — geom_run1_b_v40.txt carries no zEMCSource* keys, so they would have been enumerators that nothing could ever fill.

The PR now adds 19 identifiers rather than 22, all Tracker_FEB_* (117–135), with lastEnum at 136. isFEBTracker() uses a symbolic range so it was unaffected.

This also removes the only reason for a follow-up PR that would have registered and placed those virtual detectors, so that work is dropped rather than deferred.

One addition since your review, in VirtualDetectorMaker.cc and
constructVirtualDetectors.cc: the code that actually builds the 19 FEB
detectors, which previously had identifiers here but no implementation. It is
gated on a new key, hasTrackerFEBVirtualDetectors, defaulting false, and
no geometry file sets it. Worth flagging that this deliberately differs from the
Run1B branch, which gates the same code on TrackerHasBrassRings — a key set
true in tracker_v4 through tracker_v7, i.e. every current geometry, so on
the branch these detectors are built in nominal running. Off-by-default keeps
the capability available without changing anyone's simulation: a normal
geom_run1_b_v40.txt dump is byte-identical to the same dump taken before this
code existed.

Since that is 75 lines of C++ added after the green build test at 615cf5c, I
have re-triggered the build on the current head.

@FNALbuild

Copy link
Copy Markdown
Collaborator

⌛ The following tests have been triggered for 7523dd0: build (Build queue - API unavailable)

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 7523dd0.

Test Result Details
test with Command did not list any other PRs to include
merge Merged 7523dd0 at 5c9ca42
build (prof) Log file. Build time: 04 min 21 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
trigger Log file.
check_cmake Log file.
FIXME, TODO ➡️ TODO (0) FIXME (3) in 3 files
clang-tidy ➡️ 4 errors 242 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 7523dd0 after being merged into the base branch at 5c9ca42.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants