-
Notifications
You must be signed in to change notification settings - Fork 176
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
Conversation
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.
Codecov Report
@@ Coverage Diff @@
## main #2094 +/- ##
=======================================
Coverage ? 49.64%
=======================================
Files ? 471
Lines ? 26632
Branches ? 12237
=======================================
Hits ? 13222
Misses ? 4742
Partials ? 8668
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this 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 ?
This adds a few unit tests, and I believe those should run, yes. |
This is green once again. @asalzburger can you have a look? |
There was a problem hiding this 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.
This PR adds a self-contained PODIO plugin, independent from the EDM4hep plugin. It implements backends for the
TrackContainer
andMultiTrajectory
EDM. Tests are included for conversion of both EDM types.Blocked by: