Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: EDM4hep IO #1260

Merged
merged 114 commits into from
Jul 7, 2022
Merged

feat: EDM4hep IO #1260

merged 114 commits into from
Jul 7, 2022

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented May 18, 2022

on track

  • add EDM4hep as dependency
    • use recent acts/machines with EDM4hep
    • use recent acts/ci-dependencies with EDM4hep
  • make geometry service available in DD4hepDetector (see Examples/Detectors/DD4hepDetector/include/ActsExamples/DD4hepDetector/DD4hepDetector.hpp)
  • add EDM4hep simhit reader/writer
  • add EDM4hep particle reader/writer
  • add EDM4hep measurement reader/writer
  • add EDM4hep multi trajectory writer

off track

  • consistent namespace for DD4hepDetector
  • fix potential memory leak in Examples/Io/Root/src/RootMaterialTrackReader.cpp
  • minor reformatting

known issues

  • particle
    • ID is not persistent
    • process is not persistent
  • sim hit
    • after4 momentum is not persistent
    • hit index is not persistent
    • digitization channel is not persistent
  • measurement
    • hit index is not persistent
    • 1D coords are not persistent
    • segment path is not persistent
  • trajectory
    • trajectory state is incomplete and not persistent
    • curvature param is incorrect
    • track state local coordinates are written to (D0,Z0)
    • covariance is incorrect
    • no preserved relation to the particles

@andiwand andiwand added the 🚧 WIP Work-in-progress label May 18, 2022
@codecov
Copy link

codecov bot commented May 19, 2022

Codecov Report

Merging #1260 (cefb5b4) into main (d7c311f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1260   +/-   ##
=======================================
  Coverage   47.42%   47.42%           
=======================================
  Files         375      375           
  Lines       19788    19788           
  Branches     9287     9287           
=======================================
  Hits         9385     9385           
  Misses       4021     4021           
  Partials     6382     6382           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Contributor

@niermann999 niermann999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a brief glance, but lgtm. Only a few remarks, probably born out of ignorance :)

.github/workflows/builds.yml Outdated Show resolved Hide resolved
Examples/Io/EDM4hep/src/EDM4hepMultiTrajectoryWriter.cpp Outdated Show resolved Hide resolved
Examples/Io/EDM4hep/src/EDM4hepMultiTrajectoryWriter.cpp Outdated Show resolved Hide resolved
Examples/Io/EDM4hep/src/EDM4hepUtil.cpp Outdated Show resolved Hide resolved
Examples/Io/EDM4hep/src/EDM4hepUtil.cpp Outdated Show resolved Hide resolved
Examples/Io/EDM4hep/src/EDM4hepUtil.cpp Outdated Show resolved Hide resolved
@andiwand
Copy link
Contributor Author

andiwand commented Jul 6, 2022

maybe we can store the barcode in MCParticle::*Status and make it persistent

Copy link
Member

@paulgessinger paulgessinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kodiakhq kodiakhq bot merged commit 0955220 into acts-project:main Jul 7, 2022
@andiwand andiwand deleted the edm4hep branch July 7, 2022 12:46
@andiwand andiwand modified the milestones: next, v19.4.0 Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants