Skip to content

DDSimpleMuonDigi: build the cellID decoder once in initialize() - #48

Merged
jmcarcell merged 5 commits into
key4hep:mainfrom
TC01:muon_digi_update
Aug 25, 2026
Merged

DDSimpleMuonDigi: build the cellID decoder once in initialize()#48
jmcarcell merged 5 commits into
key4hep:mainfrom
TC01:muon_digi_update

Conversation

@TC01

@TC01 TC01 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

BEGINRELEASENOTES

  • Move the BitFieldCoder used to decode the layer out of the event loop: it is now built once in initialize() and held as a member. Suggested by @jmcarcell in review.
  • caloLayout is const

ENDRELEASENOTES

This PR originally replaced the collection-metadata lookup of the cellID encoding with a lookup of the GlobalCalorimeterReadoutID DD4hep constant, so that overlaid beam-induced background in the muon system, whose collections are created at runtime and so carry no encoding in the metadata, could be digitised.

That has been dropped, for two reasons:

  • It broke CI on every stack. CLD_o2_v07 does not define GlobalCalorimeterReadoutID (only GlobalTrackerReadoutID), so constantAsString threw and run_DDSimpleMuonDigi failed. Not every detector defines these fallback constants.
  • It only fixes one consumer. Every other algorithm reading overlay-created collections has the same problem and would need the same patch.

As a result this PR no longer fixes the overlay digitisation problem — it is now just the cleanup. The intended fix belongs at the producer: OverlayTiming and OverlayTimingRandomMix and will be followed up there.

Given #42 it's possible this shouldn't be merged here; if the desire is for digitizers to live in experiment-specific repositories we could instead make a copy of DDSimpleMuonDigi with these modifications in our fork of k4reco (at least for the time being).

@madbaron

Copy link
Copy Markdown
Member

I think that regardless of where the digitisers get moved to, we can still fix this one centrally here before it gets moved.

@jmcarcell

Copy link
Copy Markdown
Member

I'm checking why it fails for some stacks but not for others

Comment thread k4GaudiPandora/src/DDSimpleMuonDigi.cc Outdated
TC01 added 3 commits August 21, 2026 12:54
This change may be necessary (but insufficient) to make muon digitization
work with the beam induced background overlay for muon collider; as
the overlay collections don't exist at initialize time the cell ID parsing
won't work.
@jmcarcell

Copy link
Copy Markdown
Member

Alternatively the encoding can be set in initialize in the overlay algorithm and then it would be available downstream and in the resulting file; otherwise every algorithm that uses the collections has to be modified. And not every detector defines the fallback cell ID encodings

@madbaron

Copy link
Copy Markdown
Member

Alternatively the encoding can be set in initialize in the overlay algorithm and then it would be available downstream and in the resulting file; otherwise every algorithm that uses the collections has to be modified. And not every detector defines the fallback cell ID encodings

Thanks @jmcarcell, this sounds like a better fix. We'll go make the move from finalise to initialise in the overlay.
This PR now amounts to a bit of cleanup (move decoder out of event loop, drop unused variables, better reporting of error) so it's still worth merging.

@madbaron madbaron changed the title DDSimpleMuonDigi: improve cell ID parsing DDSimpleMuonDigi: build the cellID decoder once in initialize() Aug 24, 2026
@jmcarcell
jmcarcell merged commit 40bac0e into key4hep:main Aug 25, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants