EDM4hep Discussion

Tuesday 13 May 2025, 09:00 → 10:00 Europe/Zurich

Zoom

Live Notes

EDM4hep Live Notes
==================

Date: May 13, 2025
Indico: https://indico.cern.ch/event/1543312/

Connected: Juan, Tao, Thomas, Leonhard, Joseph, Juraj, Mateusz, Jacopo, Birgit, Frank, Pere

Apologies: Andre

### Upcoming workshops / conferences
https://github.com/orgs/key4hep/projects/4/views/1
    
## Progress and discussion

## Podio
* https://github.com/AIDASoft/podio/pulls
* https://github.com/AIDASoft/podio/issues
* https://github.com/orgs/AIDASoft/projects/2/views/1
 
### Merged PRs
* Store the collection information in a struct instead of a tuple [#711](https://github.com/AIDASoft/podio/pull/711)
* Restore compatibility with gcc11 [#773](https://github.com/AIDASoft/podio/pull/773)
* Clean up the ROOTReader [#777](https://github.com/AIDASoft/podio/pull/777)
* Rename NOTICE to LICENSE [#776](https://github.com/AIDASoft/podio/pull/776)

### Formalize common interface collections with a concept
* https://github.com/AIDASoft/podio/pull/758
* Better compile time checks for collection interace parts that are commonly generated but go beyond `CollectionBase`
* Make it public?
    * Currently keep it as a test to make sure that all collections provide the same interface. We have had some issues with inconsistencies in the past 
* [ ] Clarifying (documentation) comment missing
    
### Reading every library in `LD_LIBRARY_PATH` when using Python
* github.com/AIDASoft/podio/issue/770
* https://github.com/AIDASoft/podio/pull/775 
* ROOT issue: https://github.com/root-project/root/issues/18489
    * Opens every library it finds on `LD_LIBRARY_PATH`
    * Will take some time probably
* Design question: Move to free standing functions instead of `constexpr static`
    * `constexpr static` is the original issue so no easy way of having both
* Not really used at the moment except for internal name mangling
    * Not really visible in user code (only in EDM4hep and converters)
    * Open PR in k4FWCore
* Current implementation not the most "pure solution" (requires silencing of compiler warnings)
    * "Proper" solution takes more time and effort
* **Go with current solution for next tag**
    * Re-evaluate if necessary at a later stage

### RVec persistency / dictionary generation
* Push problem to ROOT?
    * Default storage in snapshot is `RVec` but can be switched to `std::vector` from ROOT 6.34 onwards (need to set corresponding option)
    * Might be possible to read these files back via podio, but should not spend a lot of effort to enable this to not give false incentive to mix Gaudi and FCCAnalyses / RDataFrame filtering / analysis
    * [ ] **Issue in ROOT to not change to `RVec` if input is `std::vector`**

### Next tag v01-03
* No more PRs / issues to be merged before

## EDM4hep
* https://github.com/key4hep/EDM4hep/pulls
* https://github.com/key4hep/EDM4hep/issues
* https://github.com/orgs/key4hep/projects/5 

### Merged PRs
* Limit the RNTuple backwards compatibility tests to podio 1.3 [#413](https://github.com/key4hep/EDM4hep/pull/413)
* Switch to the latest actions and cache build artifacts [#428](https://github.com/key4hep/EDM4hep/pull/428)

### Pointer persistency in EDM4hep
* https://github.com/key4hep/EDM4hep/issues/418
* Interplay, e.g. with Pandora that expects a pointer
    * Pandora is not touching this, just using this to transport information along
    * E.g. could be `std::any` and Handle is kept alive because `std::any` is constructed by value (via new member)
* Need to have a way for EDM4hep
    * Current workarounds would require filling a temporary vector to keep pointers to handles alive
    * No really user friendly way at the moment 
* Do we need a nice way? Should only appear in few central places
    * [ ] Document workaround with vector
    * [ ] Point uses to issue and make them indicate they use this
    * [ ] Decide on number of actual users whether it's worth the effort to do a "proper solution"

### [WiP] Replace the spin by a helicity property for the MCParticle
* https://github.com/key4hep/EDM4hep/pull/404
* Debugging special schema evolution that ideally stores `spin.z` into `helicty`
    * [x] Do we want / need this? -> Yes, at least would be worthwile to see if schema evolution can deal with this
    * Quite a few existing LCIO files with `spin`, but could be absorved in the LCIO to EDM4hep converter
    * [ ] Are there existing EDM4hep files with this?
* [ ] Need to check where this breaks downstream
* `9` as default value for "unset", why?
    * Historically what some generators have been using

### add python bindings for PIDHandler
* https://github.com/key4hep/EDM4hep/pull/397
* Some details still to be clarified(?)
* Lifetimes with `std::optional` (and others) are not behaving intuitively on the python side (with cppyy)
    * Could add throwing overloads for wrappers because there is better support for that. But naming could become confusing because we have `get_optional` in other cases and would need a different name here for the throwing version.
* Many use cases dictated by podio usage of `optional` (e.g. parametes). 
* [ ] Potentially need to discuss / decide on how our interfaces should look like in general

### Use pre-built hooks in pre-commit
* https://github.com/key4hep/EDM4hep/pull/402
* Streamlines setup
    * "Looks like other repositories"
    * clang-tidy not easily packaged (no issue for EDM4hep, but other packages use it)
* [x] Can pre-commit use a different config file than `.pre-commit-config.yaml`
    * Yes, can use the `--config` flag
    
### EDM4hep next tag v00-99-02
* To be done once podio is tagged 

## Converter & MarlinWrapper

### New nuances in keeping LCIO and EDM4hep event cotent consistent
* https://github.com/key4hep/k4MarlinWrapper/issues/235
* https://github.com/key4hep/k4MarlinWrapper/pull/236
* Attaching ParticleID objects to ReconstructedParticles (EDM4hep to LCIO) uses ObjectID (mainly collection ID) for looking up names
    * Collections that are created in function algorithms do not have a collection ID assigned
        * See: https://github.com/key4hep/k4FWCore/issues/311
    * Workaround using the wrapper internal name mapping not as straight forward as initially thought
* Does assigning collection id outside of a Frame break a contract?
    * Can no longer check whether a collection belongs to a Frame.
    * ObjectIDs main purpose is persistency. Should probably considered an implementation detail
    * Insertion into the TES or the Frame is an implementation detail as well (ownership hand-off is the defining feature)
    * Algorithm behavior might depend on whether it's called standalone or within Gaudi
* Should  collection / object id be non-public?
    * Definitely need to document what are the things that can be depended on and which ones don't
    * Expose it only for infrastructure / "expert" code that actually needs it
    * What about maps going from object to collection, e.g. for filtering on range of objects from different collections. (Actual use case?)
* [ ] Check if MarlinWrapper issues can be solved differently

## AoB

## Next meeting:
* May 27, 09:00 CET

09:00 → 09:05 Introduction¶

Speakers: Andre Sailer (CERN), Frank-Dieter Gaede (Deutsches Elektronen-Synchrotron (DE)), Thomas Madlener (Deutsches Elektronen-Synchrotron (DESY))

EDM4hep Github

Podio Github

09:50 → 09:51 Discussion¶

Speaker: Dr All