Skip to content

Fix geometry overlap: nudge dirtCentralStairwellLower3 face by 1 um - #1926

Open
oksuzian wants to merge 1 commit into
Mu2e:mainfrom
oksuzian:fix-stairwell-overlap-angle
Open

Fix geometry overlap: nudge dirtCentralStairwellLower3 face by 1 um#1926
oksuzian wants to merge 1 commit into
Mu2e:mainfrom
oksuzian:fix-stairwell-overlap-angle

Conversation

@oksuzian

@oksuzian oksuzian commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1925.

dirtCentralStairwellLower3, added in #1918, abuts CentralStairwellLowerStairs at Mu2e z = 6375.4. That neighbour is rotated by -1.5707963268 — π/2 truncated at the 10th decimal — so its face is tilted ~5e-12 rad off square and pokes into the new volume.

bin/overlapCheck.C calls CheckOverlaps(1e-12), so the artifact is reported on the default geometry (geom_common.txt):

Overlap ov00000: HallAir/dirtCentralStairwellLower30x545f8d0
  overlapping HallAir/CentralStairwellLowerStairs0x55f22b0  ovlp=3.11047e-10

This moves the abutting face out by 1 µm so the volumes no longer touch — the same remedy already applied to the neighbour itself, whose yHalfThickness carries "remove 1 um to prevent conflict as the rotation angle is not exactly pi/2".

Validation

Regenerated mu2e_common.gdml from the patched geometry via gdmldump.fcl (art exit 0) and reran overlapCheck.C:

illegal overlaps/extrusions
baseline (main @ 1891b328) 1
this change 0

Diffing the regenerated GDML against the baseline after stripping Geant4 pointer suffixes shows exactly the two twoDimVertex entries for this face:

-<twoDimVertex x="9956.8" y="-12369.8"/>
+<twoDimVertex x="9956.801" y="-12369.8"/>
-<twoDimVertex x="9956.8" y="-13589"/>
+<twoDimVertex x="9956.801" y="-13589"/>

Volume counts identical (9987 physvol), and the neighbour's rotation is untouched. No physics consequence — 1 µm on a dirt volume, against an original discrepancy of 3e-13 m.

Why not fix the rotation angle instead

I first tried replacing the truncated literal with a full-precision π/2, which also gives 0 overlaps. Two reasons I abandoned it:

  1. SimpleConfig has no π constant and no expression evaluation — values go through strtod, so M_PI/2 isn't available. It would have to be the bare literal -1.5707963267948966, which is unpleasant to read and invites someone to "tidy" it back to something shorter later.
  2. Shortening it doesn't work. I tested the intermediate -1.5707963267 (10 decimals, erring short of π/2 on the theory it would open a gap) and it still overlaps, at ovlp=5.78e-09 — worse than the original. Any inexact right angle tilts a corner in somewhere; only the full 16-digit value clears the tolerance. So the angle route is all-or-nothing on that literal.

The 1 µm nudge sidesteps the precision question entirely and matches the idiom already in these files.

Note for a follow-up

Two sibling volumes carry the same truncated literal:

  • bldg/centralStairwellUpperStairs1.txt:11
  • bldg/centralStairwellUpperStairs2.txt:11

Neither currently overlaps anything, so they're out of scope here. Worth knowing they're the same latent pattern if a new volume is ever placed against them. @sdifalco — these are your files, happy to defer if you'd rather handle the family differently.

@FNALbuild

Copy link
Copy Markdown
Collaborator

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 1cbd142: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

dirtCentralStairwellLower3, added in Mu2e#1918, abuts CentralStairwellLowerStairs
at Mu2e z = 6375.4. That neighbour is rotated by -1.5707963268, which is pi/2
truncated at the 10th decimal, so its face is tilted ~5e-12 rad off square and
pokes into the new volume.

overlapCheck.C uses CheckOverlaps(1e-12), so the artifact is reported on the
default geometry:

  Overlap ov00000: HallAir/dirtCentralStairwellLower3
    overlapping HallAir/CentralStairwellLowerStairs ovlp=3.11047e-10

Move the abutting face out by 1 um so the volumes no longer touch. This is the
same remedy already applied to the neighbour itself, whose yHalfThickness
carries "remove 1 um to prevent conflict as the rotation angle is not exactly
pi/2".

Verified by regenerating mu2e_common.gdml from the patched geometry and
rerunning overlapCheck.C: 1 illegal overlap -> 0. The regenerated GDML differs
from the baseline in exactly the two twoDimVertex entries for this face, with
identical volume counts (9987 physvol), so no other volume is perturbed.

Fixes Mu2e#1925
@oksuzian
oksuzian force-pushed the fix-stairwell-overlap-angle branch from 1cbd142 to ea88441 Compare August 9, 2026 14:54
@oksuzian oksuzian changed the title Fix geometry overlap: use full-precision pi/2 for lower stairwell rotation Fix geometry overlap: nudge dirtCentralStairwellLower3 face by 1 um Aug 9, 2026
@oksuzian

oksuzian commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

Replaced the original approach (full-precision pi/2 on the rotation literal) with a 1 um face nudge — same result, 0 overlaps, but no 16-digit constant to maintain. Force-pushed; the PR body above describes the current change. Details on why shortening the angle literal doesn't work are in the "Why not fix the rotation angle instead" section.

@oksuzian
oksuzian requested a review from sdifalco August 9, 2026 14:58
@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 1cbd142.

Test Result Details
test with Command did not list any other PRs to include
merge Merged 1cbd142 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 (0) in 0 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 1cbd142 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.

@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 ea88441: build (Build queue - API unavailable)

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at ea88441.

Test Result Details
test with Command did not list any other PRs to include
merge Merged ea88441 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 (0) in 0 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 ea88441 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.

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.

Geometry overlap after #1918: dirtCentralStairwellLower3 vs CentralStairwellLowerStairs (ovlp=3.1e-10)

3 participants