Skip to content

Add an algorithm to post-fill cellID encodings#477

Closed
tmadlener wants to merge 10 commits intomainfrom
cellid-encoding-hack
Closed

Add an algorithm to post-fill cellID encodings#477
tmadlener wants to merge 10 commits intomainfrom
cellid-encoding-hack

Conversation

@tmadlener
Copy link
Copy Markdown
Member

@tmadlener tmadlener commented Apr 16, 2026

This can be necessary if the collection patcher fills empty collections without parameters. However, in some cases it might be necessary to have a CellID encoding parameter set on these empty collections. This is a slightly hacky way of getting these encodings in.

BEGINRELEASENOTES

  • Add a CellIDEncodingFiller algorithm to set cellID encoding parameters externally

ENDRELEASENOTES

@tmadlener
Copy link
Copy Markdown
Member Author

CI failures are unrelated. However, thinking about this, this might be better placed in k4FWCore and a bit more general, something along the lines of

  • Set arbitrary parameters
  • Either in initialize or finalize
  • Guard overwriting existing parameter values

Thoughts @jmcarcell

@tmadlener
Copy link
Copy Markdown
Member Author

Absorbed #478 for now to at least make some things work. All of this will be properly solved in another way

tmadlener and others added 10 commits April 23, 2026 14:38
Don't use the to be deprecated DataHandle for the PodioDataSvc and
instead store directly into format that is understood and usable via
Functionals and IOSvc. Use new k4FWCore functionality to assign an id on
the fly.
This breaks conversion using the PodioDataSvc since the MetadataSvc does
not talk to that frame
Since due to 4c89e08c2a29b4c4b3c82bac06a59797faf2dd82 in k4FWCore, it
is not possible to set parameters in the event loop anymore.
This can be necessary if the collection patcher fills empty collections
without parameters. However, in some cases it might be necessary to have
a CellID encoding parameter set on these empty collections. This is a
slightly hacky way of getting these encodings in.
@tmadlener tmadlener force-pushed the cellid-encoding-hack branch from 5d28bae to 4b1f0ba Compare April 23, 2026 12:40
@tmadlener tmadlener closed this Apr 23, 2026
@tmadlener
Copy link
Copy Markdown
Member Author

Also absorbed #475 for now and removed the in-event parameter setting for the particle ID meta information. (I think that can be handled in a similar fashion as the CellIDEncoding, but it will be potentially more cumbersome to collect...)

@jmcarcell
Copy link
Copy Markdown
Member

jmcarcell commented Apr 27, 2026

For the CellIDEncoding, this can not be automatically discovered as it depends on what algorithms are going to run. Checking back #478 I have seen that it may not be necessary to fill the CellIDEncodings that we have in Lcio2Edm4hep.cpp now until the very end because:

  • If it's an LCIO algorithm, it will take it from the LCIO collection
  • If it's an EDM4hep algorithm, and when using EDM4hep input, it will not find it only if it's coming from a LCIO processor. However, given that the digitizers are typically the ones who set these and many are ported or are easy to port, I think for all existing cases, something that would fill these at the very end would probably work.

#478 however doesn't work because the finalize() of the tool also triggers the throw about putting during the event loop, maybe this could be studied. Alternatively, the only other way I see is to specify all of them at the beginning, but that means if you change what you run you also have to change them and we would have to change everything that uses the wrapper which is not very nice.
For the PID stuff, isn't the situation similar?
And iLCSoft/LCIO#223 won't do anything if the input is EDM4hep

@tmadlener
Copy link
Copy Markdown
Member Author

The problems usually arise when "mixing and matching" Running a full chain in LCIO and converting to EDM4hep at the end is rather straight forward. Coming from EDM4hep and then running the same chain is a bit more tricky, but would can be solved by front-loading the information (I think).

In the end, I don't think we can (and / or have to) come up with a fully automatic solution. I think for the existing large marlin based configurations this information is stable enough to just collect it once and pass it through some pre-processing step to then load it up front. Whether we store a bit more information than necessary would be a secondary concern to me.

Also we want more people to move off of the Marlin wrapper, so this might serve as another nudge ;)

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.

2 participants