Skip to content

Add missing detector type flags#994

Open
ruse-traveler wants to merge 24 commits intomainfrom
add-missing-det-type-tags
Open

Add missing detector type flags#994
ruse-traveler wants to merge 24 commits intomainfrom
add-missing-det-type-tags

Conversation

@ruse-traveler
Copy link
Contributor

@ruse-traveler ruse-traveler commented Nov 14, 2025

Briefly, what does this PR introduce?

This PR adds DD4hep DetType flags to all sensitive detectors and to beampipes. An example of how to extract these flags from calorimeter and tracker hits (where the latter includes PID output) is provided in snippets here.

Note that since only FORWARD is defined as a flag, I opted to label the FF/FB elements as ENDCAP. Then, following what was done in the central tracker, only detectors and beampipes are labeled; support and other structures are not.

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No.

Does this PR change default behavior?

No.

@github-actions github-actions bot added topic: barrel Mid-rapidity detectors topic: forward Positive-rapidity detectors (hadron-going side) topic: backward Negative-rapidity detectors (electron-going side) topic: calorimetry topic: PID Particle identification topic: far-forward Deterctors for small-angle particles topic: far-backward labels Nov 14, 2025
@veprbl
Copy link
Member

veprbl commented Nov 17, 2025

Have you tested querying these values? I don't think this works without explicit calls to dd4hep::xml::setDetectorTypeFlag() in each constructor.

@ruse-traveler
Copy link
Contributor Author

Have you tested querying these values? I don't think this works without explicit calls to dd4hep::xml::setDetectorTypeFlag() in each constructor.

Oof! I haven't tried it out yet, but I'll report back when I do! If need be, I'll do a pass to add explicit calls where needed...

@ruse-traveler
Copy link
Contributor Author

Have you tested querying these values? I don't think this works without explicit calls to dd4hep::xml::setDetectorTypeFlag() in each constructor.

Confirmed: you do indeed need an explicit call in each constructor. I just pushed a round of commits to add these calls to all sensitive detectors.

I also now have a working example of how to extract the type flags during reconstruction/analysis. I'll do a pass with it to make sure I've haven't missed/messed up anything, and then post here and add the example to either snippets or benchmarks...

@ruse-traveler
Copy link
Contributor Author

It took a minute to work through it, but it looks like things are working mostly as expected! For some reason, the type flags don't seem to be propagated for the DIRC, pfRICH, FHCal insert, and far backward detectors. I haven't figured out why yet...

detRegionRxEta epic994 py8ncdis18x275minq1Knevt1K d5m12y2025 detTypeRxEta epic994 py8ncdis18x275minq1Knevt1K d5m12y2025 detRegionRxZ epic994 py8ncdis18x275minq1Knevt1K d5m12y2025 detTypeRxZ epic994 py8ncdis18x275minq1Knevt1K d5m12y2025

@wdconinc
Copy link
Contributor

wdconinc commented Dec 6, 2025

Note that since only FORWARD is defined as a flag

Can you consider if it makes sense to upstream an addition to dd4hep? Symmetric colliders have forward in both directions, so this is EIC-inspired but in some sense of broader relevance too (for all those other asymmetric colliders in development.......).

@ruse-traveler
Copy link
Contributor Author

Note that since only FORWARD is defined as a flag

Can you consider if it makes sense to upstream an addition to dd4hep? Symmetric colliders have forward in both directions, so this is EIC-inspired but in some sense of broader relevance too (for all those other asymmetric colliders in development.......).

Yeah! I definitely think it makes sense upstream it to dd4hep! I suppose the FCC-eh proposal (or LHeC) might some other big use cases 🤷

(Though I also read a fun paper about running a muon collider with asymmetric beam energies a little bit ago...)

@veprbl
Copy link
Member

veprbl commented Dec 14, 2025

Should we propose DetFlag_FAR? To be used as DetFlag_FAR + DetFlag_FORWARD.

veprbl
veprbl previously approved these changes Dec 14, 2025
@ruse-traveler
Copy link
Contributor Author

Should we propose DetFlag_FAR? To be used as DetFlag_FAR + DetFlag_FORWARD.

I think that also makes sense! It definitely would be useful to be able to distinguish between detectors that are physically in the endcap vs. those that are along the beamline.

veprbl
veprbl previously approved these changes Dec 15, 2025
@veprbl
Copy link
Member

veprbl commented Dec 16, 2025

Acts is not happy with this one:

22:50:50    Acts           VERBOSE   --> Layer candidate match: VertexBarrel_layer\d -> VertexBarrel_layer2
22:50:50    Acts           VERBOSE   Volume: 'VertexBarrelSubAssembly' is a compound volume -> resolve the sub volumes

@ruse-traveler
Copy link
Contributor Author

Acts is not happy with this one:

22:50:50    Acts           VERBOSE   --> Layer candidate match: VertexBarrel_layer\d -> VertexBarrel_layer2
22:50:50    Acts           VERBOSE   Volume: 'VertexBarrelSubAssembly' is a compound volume -> resolve the sub volumes

Hmmm... I'm also seeing a bunch about the tagger :/

22:50:50    Acts           INFO      Translating DD4hep sub detector: Tagger2
22:50:50    Acts           VERBOSE   Processing detector element:  Tagger2
22:50:50    Acts           VERBOSE   SubDetector type is: [DetType( 0x9) : TRACKER, ENDCAP, ], compound: no
22:50:50    Acts           WARNING   Subdetector with name: 'Tagger2' has inconsistent information for translation and is not of type 'compound'. If you want to have this DetElement be translated into the tracking geometry you need add the right DetectorType or VariantParameters (at this stage the subvolume needs to be declared as BEAMPIPE or BARREl, or have a VariantParameter passive_layer=true) or if it is a compound DetElement (containing a barrel-endcap hierarchy), the type needs to be set to 'compound'.

EIC Container Build Service added 2 commits January 30, 2026 09:13
…Assembly

Adds <detectorType name="compound"/> tags to modules that act as detector assemblies to fix ACTS translation warnings about inconsistent detector information.

Changes applied to:
- Tagger1 and Tagger2 modules in taggers.xml and beamline_electron.xml
- Module entries in lumi/spec_tracker.xml
- VertexBarrelSubAssembly in definitions_craterlake.xml

These flags inform ACTS/DD4hep to treat these DetElements as compound hierarchies that require proper translation.
@ruse-traveler ruse-traveler requested a review from veprbl January 30, 2026 22:11
@ruse-traveler
Copy link
Contributor Author

All right! I think I squashed the ACTS warnings with the latest round of commits (thanks to some help from Copilot), and also resolved the merge conflict. This should be ready for another review!

@veprbl
Copy link
Member

veprbl commented Jan 30, 2026

Marking irrelevant detectors as beampipe (itself) messes with Acts geometry conversion.

name="VertexBarrelSubAssembly"
type="DD4hep_SubdetectorAssembly"
vis="TrackerSubAssemblyVis">
<detectorType name="compound"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm suspicious of these changes. Why would there only need to be a compound type for one of these assemblies?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: backward Negative-rapidity detectors (electron-going side) topic: barrel Mid-rapidity detectors topic: calorimetry topic: far-backward topic: far-forward Deterctors for small-angle particles topic: forward Positive-rapidity detectors (hadron-going side) topic: PID Particle identification topic: tracking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants