[ALICE3] Further developments for TRK#15262
[ALICE3] Further developments for TRK#15262scannito wants to merge 6 commits intoAliceO2Group:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the ALICE3 TRK segmented geometry implementation to support staggered/tilted stave placement and to compute more accurate “logical” container volumes so ROOT overlap checks behave reliably.
Changes:
- Extend segmented layer APIs to carry tilt angle and (for ML) stagger offset + configurable stave count.
- Update ML/OT layer volume construction to use analytically computed bounding radii instead of a fixed logical thickness.
- Update
Detectorsegmented configuration (hardcoded and file-driven) to pass the new geometry parameters.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
Detectors/Upgrades/ALICE3/TRK/simulation/src/TRKLayer.cxx |
Implements flipped chip placement, new bounding-radii logic, staggered ML placement, and new ctor signatures. |
Detectors/Upgrades/ALICE3/TRK/simulation/src/Detector.cxx |
Updates segmented configuration and parsing to provide tilt/stave/stagger parameters. |
Detectors/Upgrades/ALICE3/TRK/simulation/include/TRKSimulation/TRKLayer.h |
Updates layer class interfaces/members for new segmented parameters and bounding-radii overrides. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Detectors/Upgrades/ALICE3/TRK/simulation/include/TRKSimulation/TRKLayer.h
Show resolved
Hide resolved
Detectors/Upgrades/ALICE3/TRK/simulation/include/TRKSimulation/TRKLayer.h
Show resolved
Hide resolved
Detectors/Upgrades/ALICE3/TRK/simulation/include/TRKSimulation/TRKLayer.h
Show resolved
Hide resolved
| // Retrieve exact bounding boundaries automatically inherited from TRKSegmentedLayer | ||
| auto [rMin, rMax] = getBoundingRadii(sStaveWidth); | ||
|
|
||
| TGeoMedium* medAir = gGeoManager->GetMedium("TRK_AIR$"); | ||
| TGeoTube* layer = new TGeoTube(mInnerRadius - 0.333 * sLogicalVolumeThickness, mInnerRadius + 0.667 * sLogicalVolumeThickness, mLength / 2); | ||
| // TGeoTube* layer = new TGeoTube(mInnerRadius - 0.333 * sLogicalVolumeThickness, mInnerRadius + 0.667 * sLogicalVolumeThickness, mLength / 2); | ||
| TGeoTube* layer = new TGeoTube(rMin, rMax, mLength / 2); | ||
| TGeoVolume* layerVol = new TGeoVolume(mLayerName.c_str(), layer, medAir); | ||
| layerVol->SetLineColor(kYellow); | ||
|
|
||
| // Compute the number of staves | ||
| int nStaves = (int)std::ceil(mInnerRadius * 2 * TMath::Pi() / sStaveWidth); | ||
| nStaves += nStaves % 2; // Require an even number of staves |
There was a problem hiding this comment.
TRKOTLayer::createLayer() computes nStaves from circumference and ignores the configured numberOfStaves passed into the constructor (stored as mNumberOfStaves). Since the config parsing now requires nStaves for OT layers too, either use mNumberOfStaves here (and validate it) or drop the parameter from OT to avoid misleading/unused configuration.
| std::pair<float, float> TRKOTLayer::getBoundingRadii(double staveWidth) const | ||
| { | ||
| auto [radiusMin, radiusMax] = TRKSegmentedLayer::getBoundingRadii(staveWidth); | ||
|
|
||
| return {radiusMin - 0.201f, radiusMax}; |
There was a problem hiding this comment.
TRKOTLayer::getBoundingRadii() applies an unexplained - 0.201f to radiusMin. This is a hard-coded magic offset and only adjusts the inner bound, even though OT geometry already introduces an explicit half-stave shift (transRight->SetTranslation(..., 0.2, ...)). Please derive this margin from the actual OT geometry parameters (and document the units), or compute bounding radii analytically so the logical container is correct without ad-hoc constants.
This PR: