Repository navigation
feat(bic): flat imaging staves on alternating rails, carbon side walls, drawing scripts - #1200
Merged
Merged
Conversation
…rop stave offset Read the frame height and thickness once per layer before the stave loop so the stave placement can use them, guard the stave pitch for repeat=1 (a single stave is centred), and remove the unused stave offset attribute and the layer_j sign flips. No change to the geometry for the existing compact files. Co-Authored-By: Supervisor for Claude
Replace the common 10 degree tilt of the AstroPix staves by flat staves parallel to the sector bottom, sensors facing inward. Neighbouring staves alternate between a low and a high rail (EcalBarrel_StaveRail_low/high = 1.0/6.4 mm above the inner carbon plate; rails are not modelled), the first stave being low. Stave counts per layer become constants (EcalBarrel_StavesN_layer1..6 = 6,6,6,7,7,7; only layer 4 gains a stave, closing a gap in active-silicon phi coverage). The plugin throws if staves overlap on one tier, hit the outer plate or the sector walls. Drop the stave angle attribute and tilt constant. Co-Authored-By: Supervisor for Claude
… low stave Co-Authored-By: Supervisor for Claude
…faces Co-Authored-By: Supervisor for Claude
frame_outer was built with the inner plate's trapezoid half-widths, so it was about 1 mm narrower per side than the layer and did not cover the side-wall tops. Use the plate's own radial heights. Co-Authored-By: Supervisor for Claude
The wall inner face is slanted, so the outermost low stave is closest to it at its bottom outer corner. Place the outermost staves so the perpendicular distance from that corner to the inner-face plane is wall_gap, and evaluate the fit check on the same quantity. EcalBarrel_StaveWall_gap 0.2 -> 0.5 mm. Co-Authored-By: Supervisor for Claude
Add scripts/subdetector_tests/draw_bic_imaging_sector.py (x-radial cross-sections of the BIC imaging layers from the loaded TGeo geometry, with an annotated zoom at the sector wall) and scripts/subdetector_tests/render_bic_imaging_3d.py (z-buffer 3D render of selected imaging layers). Both take -c/-o and run on the normal install. Co-Authored-By: Supervisor for Claude
for more information, see https://pre-commit.ci
github-actions
Bot
added
topic: infrastructure
sly2j
requested review from
mariakzurek,
veprbl and
wdconinc
and
a balanced review from Copilot
October 28, 2026 15:12
sly2j
marked this pull request as ready for review
October 28, 2026 15:13
wdconinc
reviewed
Oct 28, 2026
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Skipping a stave prevents stave_idx from incrementing, causing subsequent staves to receive incorrect rail assignments, colors, and labels.
1 open issue
What changed in this PR
Updates the BIC imaging calorimeter geometry toward the current engineering design and adds tools to visualize the loaded geometry.
Changes:
- Replaces tilted staves with flat, alternating rail placements and configurable per-layer counts, adding a seventh stave to layer 4.
- Uses thinner carbon plates, adds side walls, and checks stave fit while preserving layer thickness.
- Adds sector cross-section drawings and 3D rendering scripts.
| File | Description |
|---|---|
src/BarrelCalorimeterImaging_geo.cpp |
Implements alternating stave placement, fit checks, and revised carbon framing. |
scripts/subdetector_tests/render_bic_imaging_3d.py |
Adds windowed 3D rendering of imaging layers. |
scripts/subdetector_tests/draw_bic_imaging_sector.py |
Adds layer cross-sections and annotated clearance views. |
compact/ecal/bic/bic.xml |
Configures rails, walls, and stave counts for all imaging layers. |
compact/ecal/bic/bic_layer1_only.xml |
Applies the updated geometry parameters to the layer-1-only configuration. |
Review effort: Balanced
Co-Authored-By: Supervisor for Claude
Skipped volumes no longer shift the rail parity (colour/label) of later staves; the per-layer counter reset and the per-stave increment now run before the skip filter. Co-Authored-By: Supervisor for Claude
wdconinc
approved these changes
Oct 28, 2026
wdconinc
left a comment
wdconinc
left a comment
Contributor
There was a problem hiding this comment.
This looks good to me. The only issue could be that the scripts are untested code that could start to diverge in assumptions from what's in the geometry. I think we can tolerate that in the scripts/ directory (or at least we currently do).
Contributor
Capybara summary for PR 1200
Material map validation
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.
Updates the BIC imaging layers (
epic_EcalBarrelImaging) towards the current engineering design. Main changes:EcalBarrel_StaveRail_low,EcalBarrel_StaveRail_gap), so they overlap in phi without touching. Sensors face inward. Stave counts are configurable per layer (6, 6, 6, 7, 7, 7); layer 4 gets a 7th stave to close a gap in active-silicon coverage.scripts/subdetector_tests/draw_bic_imaging_sector.py(sector cross-sections) andrender_bic_imaging_3d.py(3D view).Out of scope (follow-up PRs): realistic module structure; mechanical supports (trays) for the AstroPix modules.
Validation: TGeo and Geant4 overlap checks (as in CI) give 0 overlaps on craterlake, ip6_extended and inner_detector; the ACTS geometry check passes; stack height and all other layers are unchanged. Active-silicon overlap in coverage in phi between neighboring staves is 2.8 / 1.8 / 0.7 / 3.1 / 2.2 / 1.4 mm for layers 1 to 6.
Figures
What is the urgency of this PR?
What kind of change does this PR introduce?
Please check if any of the following apply
The BIC imaging geometry changes as described above (tilt removed, new frame, layer 4 has 7 staves). AI (Claude) assisted with implementation, validation and this description.