EDM4hep Discussion

→ Europe/Zurich
Zoom

Zoom

Zoom Meeting ID
98484040528
Host
Andre Sailer
Useful links
Join via phone
Zoom URL

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

Date: Oct 5, 2021
Indico: https://indico.cern.ch/event/1078340/
This is a document for taking notes during EDM4hep meetings.

Connected: Benedikt, Thomas, Placido, Juraj, Sanghyun, Valentin, Clément, Pere, Andre, Gerri

Apologies: Frank, Weidong, Tao, Xingtao

## Introduction and General Points
    
* ACAT
    * Submitted

* Lepton Photon
    * Accepted
    * https://indico.cern.ch/event/949705/

* ILCX
    * https://agenda.linearcollider.org/event/9211/abstracts/
    * deadline Oct 10
    * overview on Key4hep (adapt from Lepton Photon, Valentin)
    * k4SimDelphes dedicated contribution?
    * podio + EDM4hep separately (Thomas, submitted)


## Progress and Discussion

## Command line tools for podio
* See presentation by Juan
* print function on collections and also on objects (or `toString`).
* validation (spotting unphysical values/empty collections)?


## Aleph to EDM4hep

* Effort started, no major problem
* Some places where the data does not completely "fit"
* Need for User extensions
    * Fraction of energy in calo layers

* Sanghyun: storing waveform could be useful

## Podio

### New tag
* Make new tag before larger breaking changes are introduced
* Resolve `clone` issue (make clone return mutable)

### User data in podio
* https://github.com/AIDASoft/podio/pull/213
* **merged**
* mark as beta in the release notes
    * Test, e.g. in k4SimDelphes
* Documentation: https://github.com/AIDASoft/podio/pull/223

### Heap-use-after-free

* https://github.com/AIDASoft/podio/issues/174
* ~~Not a problem in frameworks, but if collections used outside of them~~
* Can happen depending on the order in which collections are cleared
* Deep inside the memory management of podio, so not easy to fix
* Happens more often with clang than with gcc, but could be compiler options.
    * Flagged by address-sanitizer
* Maybe requires deep changes. Change for reference counting of object classes.
    * Could be handled in writing similar to how we do with reading (`setReferences`) and "unroll" the relation structure
    * Add a dedicated type that does the reference counting outside of collection, and one that is owned by collections (without bookkeeping)
* Possible fix: https://github.com/AIDASoft/podio/pull/220
     * Introduces `MaybeSharedPtr` that manages the lifetime of the control block and that of the managed pointer separately, but **only** for mutable types (`Const` types are immutable)
     * [ ] Performance checks
     * What happens with related objects that are not yet in the collection?
     * Breaks k4LCIOReader
     * Split `clone` functionality change from the PR
     
### `clone` functionality should return mutable copies always
* See discussion https://github.com/AIDASoft/podio/issues/219


### Generic variant based approach to interfaces
* https://github.com/AIDASoft/podio/pull/215
* Overhead of the implementation?
    * `std::variant` usually compiles down to a vtable
    * Probably negligible for "real use cases"
* Python interface?
    * Need to check 
    
### Make Object ctor from Obj* private
* https://github.com/AIDASoft/podio/pull/221
* Making it private with friendship is not really viable (users would need to recompile podio all the time for small)
* Need to document that it is "not really" public

### change class names of const classes
* Make tag before this is merged
    * User data would be nice
* See presentation
* Use c++ templates for the two hierarchies?
    * More or less equivalent to using the podio templates
* Migration for already existing codebases?
    * Can we do something to help users?
    * (main) customers:
        * k4MarlinWrapper, k4SimDelphes, FCC full simulation 
    * How much effort is it? How much is hidden behind `auto`?
    * Make a separate branch with the changes to see where things break
* Benedikt: Namespaces for mutable classes can lead to very confusing situations
* Nobody objects to changes during meeting, need more feedback from people not connected
* [x] Open issue on github to collect more feedback and see where other users of podio are
    * Send email to edm4hep mailing list
    * https://github.com/AIDASoft/podio/issues/204


### multiple input sources
* see presentation https://indico.cern.ch/event/1067644/#6-podio-multiple-input-sources
* Is it enough to be able to read independent events and "move" collections from one event to another?
    * Could be done by the MultiSourceReader. Question is do we need to expose this low level functionality. How would dependencies be handled?
    * Ownership transfer of collections is definitely necessary.
    * Tracking of dependencies of collections; Get's complicated pretty soon, e.g. what happens with dependencies of dependencies?
        * User knows best
    * Need to be explicit of what are the things we guarantee and which we don't
* Direct access (aka EventHeader). Podio or edm4hep feature?
    * In principle it is EDM specific.
    * Unique "event tag" should be podio feature
* Object comparison uses ObjectID
    * Mainly used to check if we are referring to the same object
    * Up to the user to not abuse this too much
* Focus on providing the necessary building blocks to implement more complicated features

### Benchmarking

* Discuss how to integrate with validation in Key4hep


### Issues/PRs

* Fixed width integer types
    * https://github.com/key4hep/EDM4hep/issues/112
    * https://github.com/AIDASoft/podio/pull/186 for podio support, merged
    * Move to fixed width types in edm4hep?
        * Makes sense and "doesn't hurt us".
    * Doubles/floats? Are they are guaranteed to be fixed size?
        * standard defines only minimum width
        * `<cstdint>` available for floats?
            * boost has fixed width float types
            * In `std` only means to check if they are IEEE 754 compliant and width
    
* Generate an additional cmake lists file containing the generated source files
    * https://github.com/AIDASoft/podio/pull/143

#### Constness in python
* Currently solved by `Const` classes
* Also useful for `auto` type deduction, where `const auto` can be necessary otherwise to get something fully correct
* LCIO solved with flag in collections
* Do we need python writing?
    * probably yes, for writing ntuple like data

#### What are the different branches in the root file?
* https://github.com/AIDASoft/podio/issues/169
* Encode more information in the _relation_ branch names?
* Related to use in RDataFrame/RNTupe, directly looking at root file content
* Are branch names an implementation detail?
* backward compatibility "Impossible" (?)

#### Schema Evolution
- https://github.com/AIDASoft/podio/issues/86
- Discussion: https://indico.cern.ch/event/1030566/

#### Multi-Threading

#### "event class" in podio

* Currently being perceived

### Meta Data

#### Usage of "metadata" for user defined data
* need to check if current implementation addresses all use cases
* need test use-cases

### EventStore

### Features

#### framework ROOTWriter
* Could be refactored to use more standalone things via Gaudi Tools and Services
* Could potentially more easily write `std::vector`s or other "generic" types
    * Ideally put "generic" data types also in standalone podio
    * How to best allow for "extensions" to an EDM? 
    
## LCIOConverters
* https://github.com/key4hep/k4LCIOReader

## EDM4hep
https://github.com/key4hep/EDM4hep/pulls

### Add dN/dx to TrackerHit and/or Track
* https://github.com/key4hep/EDM4hep/issues/124
* dx is not really defined for a hit, a hit only has a deposited energy
* `edx` member is present in `edm4hep::TrackerHit` (but also that depends very much on reconstruction)
    * Not sure why it is present in the first place
    * Understand use cases first and postpone discussion
    * Deprecated in LCIO

### TrackerHit wrapper
* See possible solution using generic variant based wrapper above
* Look into clic reconstruction with edm4hep inputs where the need comes from
* Example of real use:
    * https://github.com/key4hep/k4MarlinWrapper/pull/39


### cellID decoding functionality in EDM4hep is missing
* https://github.com/key4hep/EDM4hep/issues/115
* pull request to fix it coming

### User defined collections as proper datatypes
* https://github.com/key4hep/EDM4hep/pull/114
* https://github.com/key4hep/EDM4hep/pull/117 (built on top of this idea)
* Type is `UserFloat` and not `float` -> APIs have to take `UserFloat`
    * A bit "suprising" that it is not possible to simply store a `float`
    * Needs proper documentation
* Something similar in FCC-edm with clash on macOS due to `Float.h` header file
* Benedikt: Be more strict? E.g. only allow to store user data if there is also metadata in the file.
    * For one parameter branch name could serve as "metadata"
    * Would make it more explicit, but also remove a bit of flexibility
    * Use utility class to enforce this
* Would be good to have the simple implementation now, including ints and doubles, and then try it out


### MCParticle Endpoint
* https://github.com/key4hep/EDM4hep/issues/113
* Keep podio simple by not allowing getters/setters to access multiple internal states. Use utility functions for this use case, if needed.
* Have to check again whether this is still relevant?

### Gaudi Documentation

* new "readthedocs" style at https://cern.ch/gaudi

### Issues

## AOB

* Problems with Gaudi HIVE and podio not being threadsafe when filling from multiple threads.
    * Can we support multiple event from multiple threads?
    * Collection is finest "granularity" that we can support probably.
    * Presentation with use case from CEPC in key4hep meeting
    * Thread safety issue is in Whiteboard that is used
    * Properly document workflows that are possible in podio (and also the ones that are actually recommended)
        * Also document our thread safety assumptions

### Next meeting:

* Oct 19

There are minutes attached to this event. Show them.
    • 09:00 → 09:05
      Introduction 5m
      Speakers: Andre Sailer (CERN), Frank-Dieter Gaede (Deutsches Elektronen-Synchrotron (DE)), Thomas Madlener (Deutsches Elektronen-Synchrotron (DESY))
    • 10:00 → 10:01
      Discussion 1m
      Speaker: Dr All