BIC: group AstroPix chips into modules (24 modules × 9 chips per stave) by wdconinc · Pull Request #1093 · eic/epic

wdconinc · 2026-04-30T00:23:30Z

Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.

This PR reorganizes the BIC (Barrel Imaging Calorimeter) AstroPix sensor placement from a flat 1D array of individual chips into a two-level hierarchy of modules (groups of 9 chips), matching the actual detector design more closely.

Motivation:

Physics fidelity: The BIC design uses physical module assemblies of 9 AstroPix chips. Grouping chips into modules in the geometry brings the simulation closer to the real detector structure.
AdePT performance: Flat, large geometry trees with hundreds of sisters at one level cause significant imbalance in AdePT's GPU-accelerated simulation. Grouping 216 chips per stave (24 modules × 9 chips) into an intermediate module level reduces the breadth of the tree and addresses this performance issue.

Geometry changes:

Individual AstroPix sensor: renamed AstroPix_Module → AstroPix_Chip
New intermediate volume: Nitrogen-filled Box AstroPix_Module containing 9 chips, 24 per stave
AstroPix_margin = 0 (chips touch within a module; was 200 µm)
Module_margin = 1.25 mm (inter-module gap) → 24 × 18.125 cm = 435 cm exactly ✓
DetElement hierarchy: stave → module_N → chip_M

Readout ID updated to system:8,sector:6,layer:4,stave:4,module:5,chip:4,slice:2,x:33:-16,y:-15 (64 bits).

Backward compatibility: ungrouped mode (module_repeat=1, the default if not specified) is preserved.

What is the urgency of this PR?

High (please describe reason below)
Medium
Low

What kind of change does this PR introduce?

Please check if any of the following apply

This PR introduces breaking changes. Please describe changes users need to make below.
This PR changes default behavior. Please describe changes below.
- The readout ID schema changes: module field width changes from 8 to 5 bits, and a new chip:4 field is added. Any reconstruction code relying on the old BIC readout schema will need to be updated.
AI was used in preparing this PR. Please describe usage below.
- GitHub Copilot CLI (Claude Sonnet 4.6) was used to implement the geometry plugin changes and XML updates.

Reorganize the BIC AstroPix chip placement from a flat 1D array into a two-level hierarchy: - chip: individual AstroPix sensor (formerly called 'module') - module: Nitrogen-filled Box grouping 9 chips, 24 per stave Geometry parameters (compact/ecal/bic/bic.xml and bic_layer1_only.xml): - Rename AstroPix_Module element -> AstroPix_Chip - AstroPix_margin = 0 (chips touch within module; was 200 um) - EcalBarrel_AstroPix_chips_per_module = 9 - EcalBarrel_AstroPix_modules_num = 24 - EcalBarrel_Module_margin = 1.25*mm (inter-module gap) - EcalBarrel_Module_length = 9 × chip_length = 18 cm - EcalBarrel_Module_pitch = 18.125 cm → 24 × 18.125 cm = 435 cm ✓ - Add module_repeat and module_dy/ny attributes to <stave>/<xy_layout> - Update readout ID: module:5,chip:4 (was module:8) system:8,sector:6,layer:4,stave:4,module:5,chip:4,slice:2,x:33:-16,y:-15 C++ plugin (src/BarrelCalorimeterImaging_geo.cpp): - Rename internal chip variables (module_volume -> chip_volume, etc.) - Parse module_repeat from <stave> and module_dy/ny from <xy_layout> - Build Nitrogen Box module volume with chip-group Assembly as its single daughter (TGeo paramVolume1D requires exactly 1 daughter in template) - Place 9 chips in the chip-group Assembly with physVolID('chip', i) - Use paramVolume1D for 24 modules per stave with physVolID('module', 0) - DetElement hierarchy: stave -> module_N -> chip_M - Add fit checks: module_dy >= module_length, occupied_length <= stave_length - Backward-compatible ungrouped mode (module_repeat=1) unchanged Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR updates the BIC (Barrel Imaging Calorimeter) AstroPix sensor geometry to introduce an intermediate module level (groups of chips) to better match detector construction and reduce geometry-tree breadth for AdePT.

Changes:

Updated the BIC geometry plugin to support optional chip grouping into modules, building a DetElement hierarchy stave → module → chip.
Updated BIC compact XML to rename the sensor module to AstroPix_Chip, define module-grouping constants (chips/module, modules/stave, module pitch), and enable grouping via module_repeat.
Updated the BIC imaging readout ID schema to include a chip field and adjust bit layout.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
`src/BarrelCalorimeterImaging_geo.cpp`	Implements grouped/ungrouped placement logic and DetElement hierarchy changes for chip modules.
`compact/ecal/bic/bic.xml`	Updates BIC geometry constants, enables module grouping, and updates the imaging readout ID.
`compact/ecal/bic/bic_layer1_only.xml`	Same as `bic.xml` but for the single-layer debug configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-30T00:31:48Z

+            // module_length = space occupied by chips inside one module (no inter-module margin)
+            double module_length     = n_chips_per_module * dy;


module_length is computed as n_chips_per_module * dy, but dy is the chip pitch (chip length + intra-chip margin). For any non-zero intra-module margin this over-counts by one margin (it effectively adds a margin after the last chip), which can (a) make the module box too long and (b) incorrectly trip the overlap/fit sanity checks. Consider computing module length as n*chip_length + (n-1)*margin (equivalently n*dy - margin) or explicitly passing/deriving the chip length separately from the pitch.

Suggested change

// module_length = space occupied by chips inside one module (no inter-module margin)

double module_length = n_chips_per_module * dy;

// module_length = space occupied by chips inside one module (no inter-module margin).

// `dy` is the chip pitch, so for grouped modules subtract the trailing intra-chip

// margin once rather than counting an extra empty gap after the last chip.

double chip_margin_y = getAttrOrDefault<double>(x_xy_layout, _Unicode(chip_margin_y), 0.0);

double module_length = n_chips_per_module * dy

- ((n_chips_per_module > 1) ? chip_margin_y : 0.0);

Copilot · 2026-04-30T00:31:49Z

+              PlacedVolume pv = chip_group.placeVolume(chip_volume, Position(x0, chip_y, 0));
+              pv.addPhysVolID("chip", i_c);
+              chip_placements.push_back(pv);
            }
-            module_element.setPlacement(module_physvol);
-            for (auto& sensitive_physvol : module_sensitives) {
-              DetElement sensitive_element(module_element, sensitive_physvol.volume().name(),
-                                           i_module);
-              sensitive_element.setPlacement(sensitive_physvol);
-              auto& sensitive_element_params =
-                  DD4hepDetectorHelper::ensureExtension<dd4hep::rec::VariantParameters>(
-                      sensitive_element);
-              sensitive_element_params.set<std::string>("axis_definitions", "XYZ");
+            // Single daughter of module Box → satisfies paramVolume1D constraint.
+            module_volume.placeVolume(chip_group);
+
+            // Place modules along the stave using a 1D parametrised volume.
+            // Module copy number (0..n_modules-1) IS the module ID.
+            double module_y0 = -(n_modules - 1) * module_dy / 2.;
+            printout(DEBUG, "BarrelCalorimeterImaging", "Stave %s: %d modules starting at y=%f mm",
+                     stave_name.c_str(), n_modules, module_y0 / dd4hep::mm);
+            PlacedVolume first_module_pv = stave_volume.paramVolume1D(
+                Transform3D(Position(0, module_y0, 0)), module_volume,
+                static_cast<size_t>(n_modules), Transform3D(Position(0, module_dy, 0)));


Grouped mode uses x0 when placing chips inside the module (chip_group.placeVolume(... Position(x0, ...))), but places the module copies at x=0 (paramVolume1D(Position(0, module_y0, 0))). If x0 is customized in XML, chips will be offset within the module box and module/child coordinates become inconsistent. It’s safer to place chips at x=0 in the module’s local frame and apply x0 only at the module placement level (mirroring the ungrouped path).

Copilot · 2026-04-30T00:31:49Z

+    <constant name="EcalBarrel_AstroPix_modules_num"      value="24"/>
+    <constant name="EcalBarrel_Module_margin"              value="1.25*mm"/>
+    <constant name="EcalBarrel_Module_length"
+      value="EcalBarrel_AstroPix_chips_per_module * (EcalBarrel_AstroPix_length + EcalBarrel_AstroPix_margin)"/>


EcalBarrel_Module_length is defined as chips_per_module * (chip_length + chip_margin), but EcalBarrel_AstroPix_margin is described as an intra-module gap between adjacent chips. That formula adds an extra margin after the last chip; the physical module length should be chips_per_module * chip_length + (chips_per_module - 1) * chip_margin (or chips_per_module * (chip_length + chip_margin) - chip_margin). This matters if the intra-chip margin is ever set non-zero and can skew EcalBarrel_Module_pitch and the stave fit check.

Suggested change

value="EcalBarrel_AstroPix_chips_per_module * (EcalBarrel_AstroPix_length + EcalBarrel_AstroPix_margin)"/>

value="EcalBarrel_AstroPix_chips_per_module * EcalBarrel_AstroPix_length + (EcalBarrel_AstroPix_chips_per_module - 1) * EcalBarrel_AstroPix_margin"/>

Copilot · 2026-04-30T00:31:49Z

+    <constant name="EcalBarrel_AstroPix_modules_num"      value="24"/>
+    <constant name="EcalBarrel_Module_margin"              value="1.25*mm"/>
+    <constant name="EcalBarrel_Module_length"
+      value="EcalBarrel_AstroPix_chips_per_module * (EcalBarrel_AstroPix_length + EcalBarrel_AstroPix_margin)"/>


EcalBarrel_Module_length is defined as chips_per_module * (chip_length + chip_margin), but EcalBarrel_AstroPix_margin is described as an intra-module gap between adjacent chips. That formula adds an extra margin after the last chip; the physical module length should be chips_per_module * chip_length + (chips_per_module - 1) * chip_margin (or chips_per_module * (chip_length + chip_margin) - chip_margin). This matters if the intra-chip margin is ever set non-zero and can skew EcalBarrel_Module_pitch and the stave fit check.

Suggested change

value="EcalBarrel_AstroPix_chips_per_module * (EcalBarrel_AstroPix_length + EcalBarrel_AstroPix_margin)"/>

value="EcalBarrel_AstroPix_chips_per_module * EcalBarrel_AstroPix_length + (EcalBarrel_AstroPix_chips_per_module - 1) * EcalBarrel_AstroPix_margin"/>

Copilot · 2026-04-30T00:31:49Z

+            // Build a Nitrogen-filled Box volume with a single chip-group Assembly daughter.
+            // TGeo requires paramVolume1D template volumes to have exactly 1 daughter.
+            std::string chip_grp_name   = stave_name + "_chip_group";
+            std::string module_vol_name = stave_name + "_module";
+            Assembly chip_group(chip_grp_name);
+            Box module_shape(stave_dim_x, module_length / 2., stave_dim_z);
+            Volume module_volume(module_vol_name, module_shape, desc.material("Nitrogen"));


desc.material("Nitrogen") is used for the new module volume, but there is no Nitrogen material defined in this repo’s compact/materials.xml (search shows no matches). As-is, geometry construction will fail at runtime when trying to resolve this material. Please either switch to an existing material (e.g. Air) or add a Nitrogen material definition to the compact materials and ensure it’s loaded before this detector is built.

Suggested change

// Build a Nitrogen-filled Box volume with a single chip-group Assembly daughter.

// TGeo requires paramVolume1D template volumes to have exactly 1 daughter.

std::string chip_grp_name = stave_name + "_chip_group";

std::string module_vol_name = stave_name + "_module";

Assembly chip_group(chip_grp_name);

Box module_shape(stave_dim_x, module_length / 2., stave_dim_z);

Volume module_volume(module_vol_name, module_shape, desc.material("Nitrogen"));

// Build an Air-filled Box volume with a single chip-group Assembly daughter.

// TGeo requires paramVolume1D template volumes to have exactly 1 daughter.

std::string chip_grp_name = stave_name + "_chip_group";

std::string module_vol_name = stave_name + "_module";

Assembly chip_group(chip_grp_name);

Box module_shape(stave_dim_x, module_length / 2., stave_dim_z);

Volume module_volume(module_vol_name, module_shape, desc.material("Air"));

wdconinc and others added 2 commits April 29, 2026 16:49

feat: switch BIC and FEMC to paramVolume1D

3bcf1e6

Copilot AI review requested due to automatic review settings April 30, 2026 00:23

github-actions Bot added topic: barrel Mid-rapidity detectors topic: calorimetry labels Apr 30, 2026

Copilot started reviewing on behalf of wdconinc April 30, 2026 00:24 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

wdconinc force-pushed the parametrized-volumes branch 3 times, most recently from efd95d0 to ad30ff6 Compare May 26, 2026 20:25

wdconinc force-pushed the parametrized-volumes branch from bb7123c to 5c941de Compare June 6, 2026 21:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BIC: group AstroPix chips into modules (24 modules × 9 chips per stave)#1093

BIC: group AstroPix chips into modules (24 modules × 9 chips per stave)#1093
wdconinc wants to merge 2 commits into
parametrized-volumesfrom
bic-module-grouping

wdconinc commented Apr 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// module_length = space occupied by chips inside one module (no inter-module margin)
		double module_length = n_chips_per_module * dy;

-            // module_length = space occupied by chips inside one module (no inter-module margin)
-            double module_length     = n_chips_per_module * dy;
+            // module_length = space occupied by chips inside one module (no inter-module margin).
+            // `dy` is the chip pitch, so for grouped modules subtract the trailing intra-chip
+            // margin once rather than counting an extra empty gap after the last chip.
+            double chip_margin_y = getAttrOrDefault<double>(x_xy_layout, _Unicode(chip_margin_y), 0.0);
+            double module_length = n_chips_per_module * dy
+                                   - ((n_chips_per_module > 1) ? chip_margin_y : 0.0);

	value="EcalBarrel_AstroPix_chips_per_module * (EcalBarrel_AstroPix_length + EcalBarrel_AstroPix_margin)"/>
	value="EcalBarrel_AstroPix_chips_per_module * EcalBarrel_AstroPix_length + (EcalBarrel_AstroPix_chips_per_module - 1) * EcalBarrel_AstroPix_margin"/>

Conversation

wdconinc commented Apr 30, 2026

Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.

What is the urgency of this PR?

What kind of change does this PR introduce?

Please check if any of the following apply

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants