Mu2eG4: fix the S0/S1 findings from the static-review audit (#1947) by oksuzian · Pull Request #1948 · Mu2e/Offline

oksuzian · 2026-08-26T18:06:09Z

Fixes the S0 and all six S1 findings from #1947 (static-review audit of Mu2eG4/). Seven commits, one per finding, so each can be reviewed and reverted on its own. Every finding was hand-verified at the source before fixing; the issue carries the full evidence and arithmetic.

commit	finding	what changes in output
EMC disk-edge VD copy number	#1947 S0	The four `EMC_Disk__Edge` VD planes now write their own volumeId (77–80) instead of 75/77; the contaminated Disk_1_SurfIn / Disk_0_EdgeIn streams become pure. Any analysis that "used" edge-VD hits before was reading mislabeled data.
`saveSimParticleEnd` boundary	#1947 S1	In events that hit `maxSimParticleCollectionSize`, the last admitted SimParticle now carries real end-of-track info instead of `endDefined()==false`.
RackSteel Fe/Al	#1947 S1	STM shielding house and electronics racks become steel instead of aluminum-at-steel-density. This shifts STM background rates — explicit STM-group sign-off requested; happy to drop this commit into its own PR if preferred.
Hayman spoke rod-side offset	#1947 S1	Back-ports the corrected expression `constructStickmanTarget` already uses (its comment: "seems to be a typo in the hayman"). Rod-side spoke-wire endpoints move by the tan²θ error factor; only the +1 mm padding is kept as-is.
OPA notch sagitta	#1947 S1	The notch cut into OPA support ring 2 shrinks from 15.18 mm to the intended 3.76 mm extension (live v04 values).
`pConePlane::operator<`	#1947 S1	The polycone duplicate-plane gate becomes a valid strict weak ordering. A geometry with genuine duplicate planes that previously slipped through will now fail loudly — that is the point.
ExtMon chiller rotation	#1947 S1	The three hall pixel chillers get the `.inverse()` every sibling call site applies; their rotation flips to the intended sign.

Validation

All seven touched files compile warning-free under g++ -std=c++20 -Wall -Wextra -fsyntax-only against the Musing include paths (the two -Wunused-parameter warnings in constructExtMonFNALInfrastructure.cc:570 are in an untouched function and present on main).
Not run: no art job, no GDML diff, no before/after geometry rendering. The behavioural claims come from the audited arithmetic. The geometry commits (VD, Hayman, OPA, chiller) deserve a gdmldump + overlap-check pass in CI, and the RackSteel commit a before/after STM rate check by someone with a suitable setup.
No FHiCL or SimpleConfig key changes anywhere, so no companion Production or mu2e-trig-config PR is required.

Deliberately not in this PR

The 29 S2 / 2 S3 findings from #1947 (dead code, dormant guards, hand-run-path bugs). Several want maintainer decisions (e.g. whether sourceProfileReader, DuplicateLogicalVolumeChecker and constructStudyEnv_v00* should exist at all) and none affects live production output.

🤖 Generated with Claude Code

…f copy number Both disk-edge nestTubs calls named the volume with vdIdDiskEdge but passed vdIdDiskSurf - by then already advanced to the next disk's SurfIn value - as the G4 copy number, which Mu2eG4SensitiveDetector stores as the StepPointMC volumeId. Disk-0 EdgeIn and EdgeOut both wrote id 75, disk-1 both wrote 77. Also restores vdIsVisible in the second call, where a literal 1 sat in the visibility slot. Mu2e#1947 finding 1 (S0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

saveSimParticleStart admits tracks while the incremented counter is <= _sizeLimit, but saveSimParticleEnd returned at >=, so the track that reached exactly _sizeLimit was persisted with a default-constructed end: endDefined()==false, trackLength -1, although G4 tracked it normally. Match the Start boundary. Mu2e#1947 finding 2 (S1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

98.5% G4_Al + 1.5% G4_C at 8.05 g/cm3 is mild steel's recipe with Fe replaced by Al - a one-token typo. RackSteel is the STM shielding house SteelMaterial (constructSTM.cc) and 66% of RackElectronics, so the simulated STM shielding was substantially more transparent than steel. Mu2e#1947 finding 3 (S1). Changes STM simulation results; needs STM group sign-off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… offset The rod-side overlap offset used tan(angle)/spokeRadius - the reciprocal of the correct expression, dimensionally 1/length used as a length. constructStickmanTarget already carries the corrected form with the comment 'seems to be a typo in the hayman'; apply the same expression here. Mu2e#1947 finding 4 (S1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The notch box spans +-notchWidth/2, so the chord-sagitta correction must use the half-width. With the live v04 values the extension was 15.18 mm instead of 3.76 mm - an ~11.4 mm oversized cut into support ring 2. Mu2e#1947 finding 5 (S1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The comparator lacked the equality guards of a lexicographic compare, so A<B and B<A could both hold and duplicate z-planes could escape the polycone validity check. Mu2e#1947 finding 6 (S1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…xpects finishNesting() uses the backwards interpretation of rotations; every sibling ExtMon call site passes rotationInParent.inverse() and documents why. The chiller mother volume passed the un-inverted rotation, placing the three hall chillers with a wrong-signed rotation. Mu2e#1947 finding 7 (S1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

FNALbuild · 2026-08-26T18:06:39Z

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

Mu2eG4

which require these tests: build.

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

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

About FNALbuild. Code review on Mu2e/Offline.

FNALbuild · 2026-08-26T18:18:57Z

☔ The build tests failed for 61546d6.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged `61546d6` at `a7a1ca9`
build (prof)	✅	Log file. Build time: 04 min 20 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. Return Code 9.
rootOverlaps	✅	Log file.
g4surfaceCheck	✅	Log file.
trigger	✅	Log file.
check_cmake	✅	Log file.
FIXME, TODO	➡️	TODO (2) FIXME (10) in 7 files
clang-tidy	➡️	11 errors 1575 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 61546d6 after being merged into the base branch at a7a1ca9.

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.

oksuzian · 2026-08-26T18:25:37Z

@FNALbuild run build test

FNALbuild · 2026-08-26T18:26:06Z

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

FNALbuild · 2026-08-26T18:38:19Z

☀️ The build tests passed at 61546d6.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged `61546d6` at `a7a1ca9`
build (prof)	✅	Log file. Build time: 04 min 22 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 (2) FIXME (10) in 7 files
clang-tidy	➡️	11 errors 1575 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 61546d6 after being merged into the base branch at a7a1ca9.

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.

gaponenko · 2026-08-26T19:12:32Z

  void Mu2eG4TrackingAction::saveSimParticleEnd(const G4Track* trk){

-    if( _sizeLimit>0 && _currentSize>=_sizeLimit ) return;
+    if( _sizeLimit>0 && _currentSize>_sizeLimit ) return;


@gaponenko, can you please also check ExtMon part?

YongyiBWu · 2026-08-26T19:52:54Z

On "S1 — Hayman target spoke offset uses an inverted, dimensionally wrong formula":
Yes this would fix the Hayman geometry. The affected spoke wires shuold not make any big change, if at all.
On "STM Racksteel":
This affects the supporting structures of STM shield house. @jpeck607 may have more comments on this.

FNALbuild · 2026-08-26T19:53:20Z

📝 The HEAD of main has changed to 8af4faf. Tests are now out of date.

gaponenko

Please revert #61546d68b53c32b85e1a6224c84b08d0469c194e
It breaks geometry.

YongyiBWu · 2026-08-26T20:04:01Z

On "STM Racksteel": Joey confirmed the materials are supposed to be steel.

…estBox expects" This reverts commit 61546d6, per review: it breaks the geometry. constructExtMonFNALChillersInHall composes its rotateY angle (with swapped wall-vector components) to match the callee un-inverted rotation use, so caller and callee are self-consistent as they stand; adding the inverse in the callee flipped the hall chillers out of their tuned orientation. The audit finding stands corrected on the issue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

oksuzian · 2026-08-26T20:11:32Z

@gaponenko Reverted in 675ab3e — thank you for catching it.

For the record of why the original claim was wrong: constructExtMonFNALChillersInHall composes its rotateY angle from already-swapped wall-vector components (rotAngle = -M_PI*0.5 - atan2(wallVector.x(), wallVector.y())), i.e. the caller's angle is built for the callee's un-inverted use of the rotation, and the pair is self-consistent as it stands. The room-chiller path passes IDENTITY, so the change moved only the hall chillers — exactly the breakage you saw. I've corrected finding 7 on #1947 accordingly. The remaining six commits are untouched.

oksuzian and others added 7 commits August 26, 2026 13:05

FNALbuild added build pending build running and removed build pending labels Aug 26, 2026

oksuzian requested review from YongyiBWu, rlcee and sdifalco August 26, 2026 18:13

FNALbuild added build failed and removed build running labels Aug 26, 2026

FNALbuild added build pending and removed build failed labels Aug 26, 2026

FNALbuild added build running and removed build pending labels Aug 26, 2026

oksuzian requested a review from gaponenko August 26, 2026 18:27

FNALbuild added build finished and removed build running labels Aug 26, 2026

gaponenko reviewed Aug 26, 2026

View reviewed changes

FNALbuild removed the build finished label Aug 26, 2026

FNALbuild added the build pending label Aug 26, 2026

oksuzian requested a review from jpeck607 August 26, 2026 19:57

gaponenko requested changes Aug 26, 2026

View reviewed changes

YongyiBWu approved these changes Aug 26, 2026

View reviewed changes

This was referenced Aug 26, 2026

Mu2eG4: static-review audit — wrong copy numbers on EMC edge virtual detectors, RackSteel built from aluminum, Hayman/OPA geometry arithmetic #1947

Open

Mu2eG4/ExtMon: clean up pixel-chiller construction rotation conventions #1950

Open

Navigation Menu

Uh oh!

Mu2eG4: fix the S0/S1 findings from the static-review audit (#1947) - #1948

Mu2eG4: fix the S0/S1 findings from the static-review audit (#1947)#1948
oksuzian wants to merge 8 commits into
Mu2e:mainfrom
oksuzian:mu2eg4-audit-fixes

oksuzian commented Aug 26, 2026

Uh oh!

FNALbuild commented Aug 26, 2026

Uh oh!

FNALbuild commented Aug 26, 2026

Uh oh!

oksuzian commented Aug 26, 2026

Uh oh!

FNALbuild commented Aug 26, 2026

Uh oh!

FNALbuild commented Aug 26, 2026

Uh oh!

gaponenko Aug 26, 2026

Uh oh!

oksuzian Aug 26, 2026

Uh oh!

YongyiBWu commented Aug 26, 2026

Uh oh!

FNALbuild commented Aug 26, 2026

Uh oh!

gaponenko left a comment

Uh oh!

YongyiBWu commented Aug 26, 2026

Uh oh!

oksuzian commented Aug 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

oksuzian commented Aug 26, 2026

Validation

Deliberately not in this PR

Uh oh!

FNALbuild commented Aug 26, 2026

Uh oh!

FNALbuild commented Aug 26, 2026

Uh oh!

oksuzian commented Aug 26, 2026

Uh oh!

FNALbuild commented Aug 26, 2026

Uh oh!

FNALbuild commented Aug 26, 2026

Uh oh!

gaponenko Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

oksuzian Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

YongyiBWu commented Aug 26, 2026

Uh oh!

FNALbuild commented Aug 26, 2026

Uh oh!

gaponenko left a comment

Choose a reason for hiding this comment

Uh oh!

YongyiBWu commented Aug 26, 2026

Uh oh!

oksuzian commented Aug 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants