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: Add PODIO plugin, Track EDM backend #2094

Merged
merged 37 commits into from
Oct 23, 2023

Conversation

paulgessinger
Copy link
Member

This PR adds a self-contained PODIO plugin, independent from the EDM4hep plugin. It implements backends for the TrackContainer and MultiTrajectory EDM. Tests are included for conversion of both EDM types.

Blocked by:

Was using `unsigned long long` before, but that's not actually
guaranteed to be `uint64_t` on all platforms
…ement access

This changes the contract between `TrackContainer`, `MultiTrajectory`
and their backends. Notably it

- Adds dedicated methods for surface manipulation that are used by the
  backend:
  - `referenceSurface_impl` to get a reference surface
    by index. It returns a pointer, and a new method
    `hasReferenceSurface` on the proxies compares it to `nullptr` to
    check if a reference surface is set
  - `setReferenceSurface_impl` to set a reference surface
  - `has_impl` for both backend types is not expected to respond to `referenceSurface` anymore.

- In `MultiTrajectory`, the backend is no longer expected to return an
  index referencing a jacobian, measurement or measurement covariance.
  Instead, the dedicated methods `jacobian_impl`, `measurement_impl` and
  `measurementCovariance_impl` are now called by the proxy with the
  state index directly, leaving the backend to implement index
  traversal. `has_impl` is not expected to respond to `calibrated`,
    `calibratedCov`, `jacobian` anymore.
This allows us to use these tests more like a compliance suite, and it
can be used to test other backend implementation (like the upcoming
PODIO one)
This PR adds a self-contained PODIO plugin, independent from the EDM4hep
plugin. It implements backends for the `TrackContainer` and
`MultiTrajectory` EDM. Tests are included for conversion of both EDM
types.
@paulgessinger paulgessinger added the 🛑 blocked This item is blocked by another item label May 5, 2023
@paulgessinger paulgessinger added this to the next milestone May 5, 2023
@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@2ed5a93). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2094   +/-   ##
=======================================
  Coverage        ?   49.64%           
=======================================
  Files           ?      471           
  Lines           ?    26632           
  Branches        ?    12237           
=======================================
  Hits            ?    13222           
  Misses          ?     4742           
  Partials        ?     8668           
Files Coverage Δ
Core/include/Acts/Surfaces/ConvexPolygonBounds.hpp 100.00% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions github-actions bot added the Stale label Jun 4, 2023
@paulgessinger paulgessinger removed Stale 🛑 blocked This item is blocked by another item labels Jun 5, 2023
@andiwand andiwand added the 🚧 WIP Work-in-progress label Jun 12, 2023
asalzburger
asalzburger previously approved these changes Aug 29, 2023
Copy link
Contributor

@asalzburger asalzburger left a comment

Choose a reason for hiding this comment

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

Is the PodIO test not run in the ?

Plugins/Podio/src/PodioUtil.cpp Outdated Show resolved Hide resolved
@paulgessinger
Copy link
Member Author

This adds a few unit tests, and I believe those should run, yes.

@paulgessinger paulgessinger marked this pull request as ready for review September 1, 2023 07:20
@paulgessinger
Copy link
Member Author

This is green once again. @asalzburger can you have a look?

@paulgessinger paulgessinger removed the 🚧 WIP Work-in-progress label Sep 4, 2023
Copy link
Contributor

@asalzburger asalzburger left a comment

Choose a reason for hiding this comment

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

I approve this, I mainly checked that the podio classes make sense, and then the test whether they fill into our ACTS classes is anyway job of the CI tests.

@kodiakhq kodiakhq bot merged commit db3ec38 into acts-project:main Oct 23, 2023
55 checks passed
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Oct 23, 2023
@paulgessinger paulgessinger modified the milestones: next, v30.3.0 Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Core Affects the Core module Component - Documentation Affects the documentation Component - Plugins Affects one or more Plugins Fails Athena tests This PR causes a failure in the Athena tests Infrastructure Changes to build tools, continous integration, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants