EDM4hep Discussion

Tuesday 7 Nov 2023, 09:00 → 10:00 Europe/Zurich

Zoom

Live Notes

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

Date: Nov 07, 2023
Indico: https://indico.cern.ch/event/1344351/

Connected: Birgit, Juraj, Swathi, Juan, Tao, Leonhard, Brieuc, Erica, Andre, Sanghyun

Apologies: Frank

## Introduction and General Points

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

* Proposal for FOSDEM (Feb 3.-4. 2024)?
   * https://codimd.web.cern.ch/X_4TmGVIR52rXJT6l-UJgQ
   * Deadline Nov 7, 17:00(?)
   
## 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

### make `Object::id` return a `podio::ObjectID`
* https://github.com/AIDASoft/podio/pull/493
* Previous version returning `unsigned` has become "useless"
* Required a few cleanups in dependencies that were relying on the `unsigned`

### Make it possible to dispose of unconsumed collection buffers
* https://github.com/AIDASoft/podio/pull/502
* Fixes leaking of buffer data for `ROOTFrameData` if collections are not consumed (affected only standalone readers).
* Cleaner implementation likely possible
   * https://github.com/AIDASoft/podio/issues/506
   
### Allow to limit which collections are read
* https://github.com/AIDASoft/podio/issues/499
* https://github.com/AIDASoft/podio/pull/504
* [ ] Interface / behavior discussion

### Use-after-free in collection destructors
* https://github.com/AIDASoft/podio/issues/492 (and https://github.com/AIDASoft/podio/issues/174)
* Reported by EIC (looks like we have to fix it after all)
* Need to find a whiteboard (and some quiet time) to try and cover all use cases

### Weird names in `dir(podio)`
* https://github.com/AIDASoft/podio/issues/512
* ROOT issue? See how this can be narrowed down.

### Split generation python and python bindings
* https://github.com/AIDASoft/podio/pull/511
* Split tests into two sets of python unittests to work around issue 512

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

### new tag v00-10-01
* https://github.com/key4hep/EDM4hep/releases/tag/v00-10-01
* No datamodel changes wrt previos tag

### Moving to double for MCParticle moment
* Moving from `float` to `double` should be straight forward and can be done
* https://github.com/key4hep/EDM4hep/pull/237
* **Breaks in quite a few places throughout key4hep!**
* How hard would it be keep the LCIO behavior?
   * Probably not too straight forward(?)
   * Option in writer might be easier to do than to touch generation
   * [ ] Open issue on podio for discussion

### edm4hep::Cluster does no have a contributions vector member
* https://github.com/key4hep/EDM4hep/issues/236
* [ ] close (and document)?
* Add it once there is a use case
* Could technically split hits to hae the same functionality
   * Potentially hard to do in EDM4hep because collections are immutable

## Converters & MarlinWrapper interplay

### Fix the conversion of edm4hep::Clusters to LCIO::Clusters
* Reported by Carsten Hensel (through a crash in a wrapped Marlin processor)
   * https://github.com/key4hep/k4EDM4hep2LcioConv/issues/35
   * https://github.com/key4hep/k4EDM4hep2LcioConv/pull/36
* Adjacent "issue" that Marlin processors might expect more from the event than simply their input collections, e.g.
   * [`IsolatedLeptonTaggingProcessor`](https://github.com/iLCSoft/MarlinReco/blob/master/Analysis/IsolatedLeptonTagging/src/IsolatedLeptonTaggingProcessor.cc) states ReconstructedParticle and Vertex, but also operates on Tracks and Clusters of reco particles (without further checks)
   * Pre-existing in Marlin where not reading all necessary data leads to the same issue 
   
### Removal of legacy EDM4hep support
* Partial support up until now
* https://github.com/key4hep/k4EDM4hep2LcioConv/pull/38
   

## AoB

### SkipEventException not handled correctly
* https://github.com/key4hep/k4MarlinWrapper/issues/158
* Two issues in one: 
  * PodioOutput not handling things nicely (https://github.com/key4hep/k4MarlinWrapper/issues/94) 
  * MarlinWrapper not handling he exception correctly(?)
* PodioOutput ideally states which collections are missing but terminates(?)
  * Needs some work in podio and FWCore
  
### EDM4hep not building on top of latest podio in LCG stacks
* Code generation doesn't find ROOT when importing the code generation script
  
  https://lcgapp-services.cern.ch/cdash/viewFiles.php?buildid=279319
```
  -- Creating 'edm4hep' datamodel
Traceback (most recent call last):
 File "/build/jenkins/workspace/lcg_mr_pipeline/install/dev4/podio/00.17.01/x86_64-centos7-gcc11-opt/python/podio_class_generator.py", line 17, in <module>
   from podio_schema_evolution import DataModelComparator  # dealing with cyclic imports
 File "/build/jenkins/workspace/lcg_mr_pipeline/install/dev4/podio/00.17.01/x86_64-centos7-gcc11-opt/python/podio_schema_evolution.py", line 8, in <module>
   from podio.podio_config_reader import PodioConfigReader
 File "/build/jenkins/workspace/lcg_mr_pipeline/install/dev4/podio/00.17.01/x86_64-centos7-gcc11-opt/python/podio/__init__.py", line 8, in <module>
   import ROOT  # pylint: disable=wrong-import-order
ModuleNotFoundError: No module named 'ROOT'
CMake Error at /build/jenkins/workspace/lcg_mr_pipeline/install/dev4/podio/00.17.01/x86_64-centos7-gcc11-opt/lib64/cmake/podio/podioMacros.cmake:210 (message):
 Could not generate datamodel 'edm4hep'.  Check your definition in
 '../edm4hep.yaml'
Call Stack (most recent call first):
 edm4hep/CMakeLists.txt:4 (PODIO_GENERATE_DATAMODEL)
```
* Check if things can be fixed with https://github.com/AIDASoft/podio/pull/511 and re-tag if that is the case
  
## Next meeting:
* Nov 21, 09:00

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:45 → 09:46 Discussion

Speaker: Dr All