Repository navigation
Geomfixafterai - #2031
Geomfixafterai#2031
Conversation
|
☀️ The build tests passed at 3147d0a.
N.B. These results were obtained from a build of this Pull Request at 3147d0a after being merged into the base branch at 24a1aa5. For more information, please check the job page here. |
oksuzian
left a comment
oksuzian
left a comment
There was a problem hiding this comment.
PR Review Summary — #2031
Reviewed at head 3147d0a0ae06d90f2155fec678aa1d5ab6c64d23. first pass
Decision
- 🟡 comment only. Thank you for working through #2017. Every fix I could map to an audit item lands on the drawn value. There is one problem, which CI cannot see: the remote-handling hatch commit leaves a sealed 0.1 m³ air pocket in the overburden (finding 1). The fix is one vertex, and I have tested it. With that in, I would approve.
Scope understood
- 13 commits, each fixing one item from #2017, across 72 files in
Mu2eG4/geom/bldg:- 30 new volumes, listed in
mu2eHall_v03.txt; tsDsAreaWallinsertN(_v02).txtdeleted, with that space now inside a tallerpsArea.ceiling.
- 30 new volumes, listed in
- This is live everywhere:
geom_run1,geom_run1_a,geom_run1_a_stickman(and sogeom_common) and the2021_PhaseIfiles all loadmu2eHall_v04→mu2eHall_v03. - How I checked:
- I parsed every hall solid at the merge base (
24a1aa50, main) and at this head. - I compared them exactly, one y-slab at a time, with polygon booleans (shapely).
- I probed the material column at each changed region.
- All coordinates below are the files' own polygon coordinates, with y measured from the floor (721'-0").
- I parsed every hall solid at the merge base (
Findings
-
🟠 [S1] The RHR hatch base fix leaves a sealed 0.1 m³ air pocket in
dirt.beamline.slab.W2- Evidence:
- In
dirtBeamlineSlabW2.txt, commit638ca4acdmoves theSE corner rem hatchvertex from (9550.51, 13055.59) to (9550.51, 12598.4). - The polygon now runs straight from
d1(11684, 14406.98) to the new corner. That cuts off the triangle (11684, 14406.98)–(9550.51, 13055.59)–(9550.51, 12598.4), which is 487,705 mm² in plan. - Nothing else covers that triangle in this layer:
dirt.beamline.slab.NCstops at its (9550.51, 13055.59)–d1edge;- the new
remoteHandlingHatchLowerSWstops at x = 9550.4.
- At (10600, 13500) the column reads
dirt.beamline.upperat y = 5800, air at y = 5900–6090 (dirt.beamline.slab.W2on main), anddirt.beamline.slab2.NWat 6150. - That makes a 0.099 m³ void sealed between dirt layers at 740'-4" to 741'-0".
- In
- Impact: a pocket of missing overburden next to the RHR hatch. The amount is small for shielding, but it is the same mechanism as the 0.77 m³ pocket in #2014.
g4surfaceCheckandrootOverlapscannot report it, because they look for intersecting solids, not missing ones. - Suggested fix: in
dirtBeamlineSlabW2.txt, keep the old corner and add the new one. Put9550.51/13055.59back betweend1and the new (9550.51, 12598.4) entry, in bothxPositionsandyPositions. I applied this to a copy of the head:- the triangle is then fully covered;
- the polygon is valid;
- there are no overlaps with any hall solid at that height.
- Evidence:
-
🟡 [S2] Splitting volumes undoes the air-out in the surface test-stand geometries
- Evidence:
geom_Wideband4modules_v3/v17/v18/v19,geom_WidebandMuonTaggers_v01,geom_common_trackerVSTandgeom_common_trackerStationVSTset most hall volumes toG4_AIR, for examplebuilding.escape.hatch.N,building.firstFloor.S,building.dsArea.elevator.ceiling,building.remote.handling.Hatch.Lower,building.tsArea.stairwellandbuilding.firstFloor.wall.- Twelve new concrete volumes are pieces of those aired volumes, and none of them is aired:
escape.hatch.NE/NTop/NBot;dsArea.firstFloor.SE;dsArea.elevator.ceilingS;remote.handling.Hatch.Lower.SW;tsArea.stairwell.E;firstFloor.wall.lower1–5.
CRVResponse/test/wideband/wideband4modules.fclloadsgeom_Wideband4modules_v19.txt.
- In six of the seven files the same happens to 11 new dirt volumes, for example
dirt.dsArea.upper2–4anddirt.firstFloor.lower.1–5. The exception is_v3.
- Evidence:
- Impact: in those test-stand simulations, part of the escape hatch, the elevator ceiling and the other volumes above become concrete or dirt again. No production campaign uses these geometries.
- Suggested fix: add
string <prefix>.material = "G4_AIR";for the twelve new prefixes in the seven files, next to their siblings.
- ⚪ [S3] Two small items:
mu2eHall.txtandmu2eHall_v02.txtstill#includethe deletedtsDsAreaWallinsertN.txt, and four test-stand geometries still setbuilding.tsDsArea.wallinsert.N.material. Neither hall file loads even at main (4 and 15 missing includes), so nothing that runs is affected.- DocDB 58456 (v2, the version I could read) documents A1–A9 only. The B and C fixes here (B5, B6, B8, B9, B10, C2, C3) and the central-stairwell rework are not described there. A short addition would let the next reader trace them.
🟢 Verified at this head, against the drawn values in #2017:
- A1, DS hatch ledges. Each ledge gains a 5" strip, so F/F at 737'-6"–740'-6" is now 21'-10", as SC-4 prints. That adds 5.26 m³.
- A3, trench floors. The DS and TS trench floors now run 717'-0" to 718'-0", which is B/TRENCH EL 718'-0". That adds 4.15 m³.
- A5, W23 opening. It is 1028.7 mm × 2197.1 mm (3'-4½" × 7'-2½"), from 721'-0" to 728'-2½".
- A6, M4 escape-shaft door. It is 3'-4½" wide and 7'-2½" high, from 746'-0" to 753'-2½" (
escapeHatchNBot/NTop). The sill at outdoor grade is your assumption, stated in DocDB 58456; the drawing does not print it. - A7, S8 soffit. The soffit is now at 731'-10" over the 4'-11½" strip, which converts 1.17 m³.
- B5, slab-on-grade. It is now 6" thick (746'-0" to 746'-6"), which puts 36 m³ back to fill. The S10 strip goes from 16" to 12", another 0.83 m³.
- B6, W22 downstand. It is now 742'-6" to 745'-2" in five segments, so about 11.1 m³ of dirt becomes concrete. The segment ends meet the counterforts.
- B8, RHR hatch upturns. They run 738'-2" to 741'-0", so about 4 m³ of dirt becomes concrete. Finding 1 is the one side-effect.
- B10, B14 strip. It is now concrete, 0.74 m × 2.44 m from 741'-0" to 745'-2" (2.3 m³), through the taller
psArea.ceiling. The removedwallinsert.Nspace is fully insidepsArea.ceiling, with no air left behind. - C2, B1 beams. All five move 7" (177.8 mm) west.
- C3, W17 walls. Both east ends are 10" (250.7 mm) shorter, now dirt.
- Leftover gaps. Apart from finding 1, the remaining slivers along the new seams are 0.03–0.3 mm wide, the same as the tolerance gaps already in these files. The
central.stairwell.lower.wall.n2raise also closes a 70 mm air gap that was on main. - Volume list.
- All 414 included
bldgfiles define a listed prefix, and every listed prefix is defined. - Materials are unchanged (
CONCRETE_MARS,MBOverburden, and so on). - The 384 base and 413 head solids all parse, with no missing keys.
- All 414 included
- CI. FNALbuild passes at this head, including
g4surfaceCheckandrootOverlaps.triggerfails with return code 2, as it has on #2021, #2026, #2029 and #2030, so that failure does not come from this PR.
Not addressed here (for the record, not findings)
- A2 and A4 (elevator pit, sump). DocDB 58456 says they were omitted on purpose.
- A8 and A9 (absorber) are outside this PR.
- B1 (W2 is 2'-11" over its full height, about 19 m³ of concrete on the soil side). The A1 fix extends the ledge inboard rather than thinning the wall, so the interior F/F now matches but the exterior face is still 5" proud.
- B2, B3, B4, B7, B11 and C1.
- The S13 edge over W14 in B10 (0.3 m³).
Validation check
- Build/tests run:
- FNALbuild: passed, as above.
- Locally: an exact per-slab base-vs-head polygon diff, material probes, and the vertex fix for finding 1. Scripts are
claude-scratch/pr2031_review/{run_parse,run_slabs,run_probe,run_probe2,run_wedge,fixcheck}.sh. - Not checked:
- the B9 beam's span against the drawing. It now runs from polygon x = 6121 to 29083, which is longer than the grid C–E span I quoted, and the volume is 4.3 m³ against my 2–2.7 m³ estimate;
- the central-stairwell rework (
3147d0a0a: walls, the new upper ceiling, and the dirt around them) against SC-19.
- Config contract check: pass. Every included file is listed, and every listed prefix is defined.
- Cross-repo consistency: no other repository uses these files.
Author follow-ups
- Restore the (9550.51, 13055.59) vertex in
dirtBeamlineSlabW2.txt(finding 1). - Add the
G4_AIRoverrides for the twelve new volumes in the test-stand geometries (finding 2). - Optionally, drop the dangling
wallinsertreferences and extend DocDB 58456 to cover the B and C fixes (finding 3).
|
doc-db 58456 has been updated, latest version is version 5 |
|
@FNALbuild run build test |
|
⌛ The following tests have been triggered for 9df3785: build (Build queue - API unavailable) |
|
☀️ The build tests passed at 9df3785.
N.B. These results were obtained from a build of this Pull Request at 9df3785 after being merged into the base branch at 24a1aa5. For more information, please check the job page here. |
oksuzian
left a comment
oksuzian
left a comment
There was a problem hiding this comment.
PR Review Summary — #2031
Reviewed at head 9df37859ad7f58e244e93d9b3b1464f5f1d384be. re-review of 3147d0a0
Decision
- 🟢 approve. The air pocket is closed, and the rest of the review stands as before. The two remaining items are minor and need not hold this up.
Findings
- 🟡 [S2] The surface test-stand geometries do not air out the new volumes (carried over, unchanged). Twelve new concrete pieces, for example
escape.hatch.NE/NTop/NBot,firstFloor.wall.lower1–5andremote.handling.Hatch.Lower.SW, are not set toG4_AIRin the seven Wideband/VST geometries that air out their parent volumes. In six of those files, the same holds for 11 new dirt volumes. No production campaign uses these geometries; this can be a follow-up. - ⚪ [S3]
mu2eHall.txtandmu2eHall_v02.txtstill include the deletedtsDsAreaWallinsertN.txt(carried over). Neither file loads at main either, so nothing is affected. Thanks for extending DocDB 58456 (v5); I have not re-read it.
🟢 Fixed in 9df37859, verified:
- [was S1] The sealed air pocket in
dirt.beamline.slab.W2. The (9550.51, 13055.59) vertex is back, betweend1and the new (9550.51, 12598.4) corner, andyreplacecorrectly moves from {9, 10} to {10, 11}.- I re-ran the exact per-slab base-vs-head diff at this head. The 487,755 mm² SOLID→AIR region at 740'-4" to 741'-0" is gone. That layer now only changes material where the RHR upturns were meant to go, from dirt to concrete.
- The commit introduced no other change in any slab.
- At (10600, 13500), y = 5950–6050, the column reads
dirt.beamline.slab.W2again, as on main. - All 413 head solids parse, and the polygon is valid.
- Everything else verified in my first pass at
3147d0a0is unchanged by this commit, which touches onlydirtBeamlineSlabW2.txt:- the A1, A3, A5, A6, A7, B5, B6, B8, B10, C2 and C3 fixes;
- the volume list;
- the deleted-file includes.
Validation check
- Build/tests run:
- FNALbuild at this head: passed, including
g4surfaceCheckandrootOverlaps.triggerfails with return code 2, as it has on every Offline PR build since #2026, so that does not come from this PR. - Locally: an exact per-slab polygon diff and material probes at this head (
claude-scratch/pr2031_r2/run.sh).
- FNALbuild at this head: passed, including
- Config contract check: pass.
- Cross-repo consistency: no other repository uses these files.
Author follow-ups
- Optional: the
G4_AIRoverrides for the new volumes in the test-stand geometries (finding 1).
|
concerning the S2 comment the following lines should be added for example to geom_Wideband4modules_v19.txt: string dirt.beamline.slab3.NE.material = "G4_AIR"; I tried to build a new wideband gdml file but wideband4modules.fcl looks obsolete. It gives errors in some module parameter configuration (some parameter initialization is missing, other parrameters are obsolete). So I just leave an example of how to update a wideband geometry file setting to air the new volumes created by this PR. |
oksuzian
left a comment
oksuzian
left a comment
There was a problem hiding this comment.
PR Review Summary — #2031
Reviewed at head 8a8b4008b10825f084d06987710b38046fe01cac. re-review of 9df37859
Decision
- 🟡 comment only, for now. Thanks for the worked example. It covers everything finding 2 asked for in that file. No FNALbuild run exists at this head, so I'm triggering one and will approve once it is green.
Findings
- ⚪ [S3] The other six test-stand geometries still need the same block (finding 2 of my earlier review, now narrowed).
geom_Wideband4modules_v3/v17/v18,geom_WidebandMuonTaggers_v01,geom_common_trackerVSTandgeom_common_trackerStationVSTdo not yet air out the new volumes. Your note thatwideband4modules.fclno longer runs on current Offline explains why_v19alone was updated. I agree this can wait for whoever next uses those stands.
🟢 Verified at this head:
- The
_v19block is complete. This PR adds 30 new hall volumes, and the 29 lines ingeom_Wideband4modules_v19.txtair out 29 of them: every newbuilding.*piece except one, and every newdirt.*piece. None of the 29 names is a typo or a volume that does not exist. - The one omission is consistent.
building.central.stairwell.upper.ceilingis not aired, but_v19does not air any of the existing central-stairwell volumes either (walls, landings, stairs,dirt.central.stairwell.*). Leaving it as concrete matches its siblings. - Nothing else changed. The new commit touches only
geom_Wideband4modules_v19.txt. The hall geometry is the same as at9df37859, which I diffed slab by slab in my previous pass.
Validation check
- Build/tests run: the solid lists for base and this head were parsed and compared, and the new
G4_AIRlines were matched against the new volume prefixes (claude-scratch/pr2031_r2/). FNALbuild passed at9df37859. This head changes only a test-stand geometry that no CI job loads. - Config contract check: pass.
- Cross-repo consistency: no other repository uses these files.
Author follow-ups
- Optional: the same block in the other six test-stand geometries when they are next used (finding 1).
|
@FNALbuild run build test |
|
⌛ The following tests have been triggered for 8a8b400: build (Build queue - API unavailable) |
|
☀️ The build tests passed at 8a8b400.
N.B. These results were obtained from a build of this Pull Request at 8a8b400 after being merged into the base branch at 24a1aa5. For more information, please check the job page here. |
oksuzian
left a comment
oksuzian
left a comment
There was a problem hiding this comment.
PR Review Summary — #2031
Reviewed at head 8a8b4008b10825f084d06987710b38046fe01cac. re-review of 9df37859
Decision
- 🟢 approve. Thanks for the worked example. It covers everything finding 2 asked for in that file.
Findings
- ⚪ [S3] The other six test-stand geometries still need the same block (finding 2 of my earlier review, now narrowed).
geom_Wideband4modules_v3/v17/v18,geom_WidebandMuonTaggers_v01,geom_common_trackerVSTandgeom_common_trackerStationVSTdo not yet air out the new volumes. Your note thatwideband4modules.fclno longer runs on current Offline explains why_v19alone was updated. I agree this can wait for whoever next uses those stands.
🟢 Verified at this head:
- The
_v19block is complete. This PR adds 30 new hall volumes, and the 29 lines ingeom_Wideband4modules_v19.txtair out 29 of them: every newbuilding.*piece except one, and every newdirt.*piece. None of the 29 names is a typo or a volume that does not exist. - The one omission is consistent.
building.central.stairwell.upper.ceilingis not aired, but_v19does not air any of the existing central-stairwell volumes either (walls, landings, stairs,dirt.central.stairwell.*). Leaving it as concrete matches its siblings. - Nothing else changed. The new commit touches only
geom_Wideband4modules_v19.txt. The hall geometry is the same as at9df37859, which I diffed slab by slab in my previous pass.
Validation check
- Build/tests run: the solid lists for base and this head were parsed and compared, and the new
G4_AIRlines were matched against the new volume prefixes (claude-scratch/pr2031_r2/). FNALbuild passed at this head.triggerfails with return code 2, as it has on every Offline PR build since #2026, so that does not come from this PR. - Config contract check: pass.
- Cross-repo consistency: no other repository uses these files.
Author follow-ups
- Optional: the same block in the other six test-stand geometries when they are next used (finding 1).
|
after my last commit I think that the PR can be merged since Andrei has asked to postpone the fixes to the proton beam dump |
oksuzian
left a comment
oksuzian
left a comment
There was a problem hiding this comment.
PR Review Summary — #2031
Reviewed at head d7baa4677e33ea4922b54904477517a85547f8d8. re-review of 8a8b4008
Decision
- 🟡 comment for now. The code is approvable: the new commit is outside the hall geometry I checked before, and it does what it says for the current geometry. I'd still like the pump removal moved into the geometry file (finding 1), but that need not hold up the merge. There is no build at this head yet, so I am triggering one and will approve when it is green.
Scope understood
- One new commit,
d7baa467"forgot commit from PR 1918". It touches onlyMu2eG4/src/constructServicesGeom.cc. It adds a hard-coded skip,if (!isBend && it== 2 && ( ip == 0 || ip == 2 ) ) continue;, labelled "skip obsolete pump pipes", plus whitespace changes. - No hall geometry file changed, so the per-slab diff and the
_v19air-out check from my last pass still hold.
Findings
-
🟡 [S2] The two pumps are removed by list position in C++, while the geometry files still declare them.
- Evidence: in
PipeMaker.cc,it == 2isPipe.*Type3, so the skip dropsType3Pipe1andType3Pipe3. InPipe_v04.txt(included bygeom_run1_a_stickman.txt, the currentgeom_common.txt) these are the two diffusion pumps at z = 14600 (x = −2664 and −5144). The file still hasPipe.nPipeType3 = 4and all four centres. Nothing else refers to the removed volume names. - Impact on older geometries: the skip runs for every geometry with a straight type 3, not just
Pipe_v04.Pipe_v03has the same pump list, so it loses the same two pumps.- In
Pipe_v01andPipe_v02, type 3 is a single 12.7 m G10Lite line ("12 in PS-TS_Shield.txt"). The skip removes that line, which is not a pump at all. Pipe_v01is used bygeom_common_cd3_s4p2, which five other geometry files still include.- I found no campaign that runs these older geometries, hence S2.
- Impact on readers: anyone reading
Pipe_v04.txtsees four pumps but gets two. The reason the pumps are obsolete is not in the commit, the PR body or DocDB 58456. - Why the skip is not needed by the hall changes: I checked all four pump footprints (1700 mm long, 260 mm radius) against every hall building and dirt solid, at both
mainand this head. Neither version has an overlap. - On the commit title: PR #1918's six commits never touched
constructServicesGeom.cc. The title therefore doesn't explain where this change comes from. - Suggested fix: revert the C++ skip. Add a
Pipe_v05.txtwithPipe.nPipeType3 = 2and only the two pumps that remain, with a one-line comment saying why the other two are obsolete. Include it fromgeom_run1_a_stickman.txt. Old geometries then keep what they had.
- Evidence: in
-
⚪ [S3] The commit re-indents the two
pipeType/pipeLogicVolTypename lines by two extra spaces and removes a blank line. This is cosmetic; finding 1's revert would drop it anyway.
🟢 Verified at this head:
- The hall geometry is unchanged since
8a8b4008. The compare lists one file,constructServicesGeom.cc. The slab-by-slab diff and the 29-lineG4_AIRblock ingeom_Wideband4modules_v19.txtfrom my last review still apply. - No other code depends on the removed volumes.
git grepforType3Pipe1,Type3Pipe3,pipeType3andpipeLogicVolType3outside thePipe_v0x.txtfiles finds nothing. - The proton beam dump. I note your comment that Andrei asked for the beam-dump fixes to be postponed.
Validation check
-
Build/tests run: no FNALbuild run at this head yet (
mu2e/buildtest: "This test has not been triggered yet"). The previous head8a8b4008passed. Locally:- read the increment;
- checked the type-to-index mapping in
PipeMaker.cc; - checked which geometries include each
Pipe_v0x.txt; - checked the pump footprints against the parsed hall solids at base and head (
claude-scratch/pr2031_r3/pumps2.sh).
I did not run Geant4 or an overlap check with the pumps removed.
-
Config contract check: pass.
-
Cross-repo consistency: pass. No other repository builds these volumes.
Residual risk
- Small: two steel pump shells with oil filling are removed from the current geometry. I can't judge from the PR whether that is the intended physics change, because the reason isn't recorded.
Author follow-ups
- Say why the two pumps at z = 14600 are obsolete, and where that comes from.
- Optional: move the removal into a new
Pipe_v05.txt, so the older geometries are unchanged (finding 1).
|
@FNALbuild run build test |
|
⌛ The following tests have been triggered for d7baa46: build (Build queue - API unavailable) |
|
☀️ The build tests passed at d7baa46.
N.B. These results were obtained from a build of this Pull Request at d7baa46 after being merged into the base branch at f122c5b. For more information, please check the job page here. |
oksuzian
left a comment
oksuzian
left a comment
There was a problem hiding this comment.
PR Review Summary — #2031
Reviewed at head d7baa4677e33ea4922b54904477517a85547f8d8. re-review of 8a8b4008
Decision
- 🟢 approve. The new commit is outside the hall geometry I checked before, and it does what it says for the current geometry. I'd still like the pump removal moved into the geometry file (finding 1), but that need not hold up the merge.
Scope understood
- One new commit,
d7baa467"forgot commit from PR 1918". It touches onlyMu2eG4/src/constructServicesGeom.cc. It adds a hard-coded skip,if (!isBend && it== 2 && ( ip == 0 || ip == 2 ) ) continue;, labelled "skip obsolete pump pipes", plus whitespace changes. - No hall geometry file changed, so the per-slab diff and the
_v19air-out check from my last pass still hold.
Findings
-
🟡 [S2] The two pumps are removed by list position in C++, while the geometry files still declare them.
- Evidence: in
PipeMaker.cc,it == 2isPipe.*Type3, so the skip dropsType3Pipe1andType3Pipe3. InPipe_v04.txt(included bygeom_run1_a_stickman.txt, the currentgeom_common.txt) these are the two diffusion pumps at z = 14600 (x = −2664 and −5144). The file still hasPipe.nPipeType3 = 4and all four centres. Nothing else refers to the removed volume names. - Impact on older geometries: the skip runs for every geometry with a straight type 3, not just
Pipe_v04.Pipe_v03has the same pump list, so it loses the same two pumps.- In
Pipe_v01andPipe_v02, type 3 is a single 12.7 m G10Lite line ("12 in PS-TS_Shield.txt"). The skip removes that line, which is not a pump at all. Pipe_v01is used bygeom_common_cd3_s4p2, which five other geometry files still include.- I found no campaign that runs these older geometries, hence S2.
- Impact on readers: anyone reading
Pipe_v04.txtsees four pumps but gets two. The reason the pumps are obsolete is not in the commit, the PR body or DocDB 58456. - Why the skip is not needed by the hall changes: I checked all four pump footprints (1700 mm long, 260 mm radius) against every hall building and dirt solid, at both
mainand this head. Neither version has an overlap. - On the commit title: PR #1918's six commits never touched
constructServicesGeom.cc. The title therefore doesn't explain where this change comes from. - Suggested fix: revert the C++ skip. Add a
Pipe_v05.txtwithPipe.nPipeType3 = 2and only the two pumps that remain, with a one-line comment saying why the other two are obsolete. Include it fromgeom_run1_a_stickman.txt. Old geometries then keep what they had.
- Evidence: in
-
⚪ [S3] The commit re-indents the two
pipeType/pipeLogicVolTypename lines by two extra spaces and removes a blank line. This is cosmetic; finding 1's revert would drop it anyway.
🟢 Verified at this head:
- The hall geometry is unchanged since
8a8b4008. The compare lists one file,constructServicesGeom.cc. The slab-by-slab diff and the 29-lineG4_AIRblock ingeom_Wideband4modules_v19.txtfrom my last review still apply. - No other code depends on the removed volumes.
git grepforType3Pipe1,Type3Pipe3,pipeType3andpipeLogicVolType3outside thePipe_v0x.txtfiles finds nothing. - The proton beam dump. I note your comment that Andrei asked for the beam-dump fixes to be postponed.
Validation check
-
Build/tests run: FNALbuild passed at this head (build 3462, merged at
f122c5b3), includingrootOverlaps,g4surfaceCheck,ceSimReco,cosmicSimRecoandtrigger. clang-tidy reports 1 error and 9 warnings; I did not read its log. Locally:- read the increment;
- checked the type-to-index mapping in
PipeMaker.cc; - checked which geometries include each
Pipe_v0x.txt; - checked the pump footprints against the parsed hall solids at base and head (
claude-scratch/pr2031_r3/pumps2.sh).
I did not run Geant4 or an overlap check with the pumps removed.
-
Config contract check: pass.
-
Cross-repo consistency: pass. No other repository builds these volumes.
Residual risk
- Small: two steel pump shells with oil filling are removed from the current geometry. I can't judge from the PR whether that is the intended physics change, because the reason isn't recorded.
Author follow-ups
- Say why the two pumps at z = 14600 are obsolete, and where that comes from.
- Optional: move the removal into a new
Pipe_v05.txt, so the older geometries are unchanged (finding 1).
This PR contains the geometry fixes added after the evaluation of the AI review of geometry described in github issue #2017.
A detailed explanation of the changes can be found in doc-db 58456