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

refactor: Add concepts for Track(State) backends #2093

Merged
merged 12 commits into from
May 24, 2023

Conversation

paulgessinger
Copy link
Member

This PR adds concepts for the MultiTrajectory and TrackContainer backends. It also centralizes the index typedef and kInvalid sentinel value definition.

Blocked by:

…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.
@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
@paulgessinger
Copy link
Member Author

/cc @benjaminhuth

@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Merging #2093 (c85abba) into main (f7ccfee) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2093   +/-   ##
=======================================
  Coverage   49.46%   49.46%           
=======================================
  Files         434      434           
  Lines       25015    25015           
  Branches    11525    11525           
=======================================
  Hits        12374    12374           
  Misses       4489     4489           
  Partials     8152     8152           
Impacted Files Coverage Δ
Core/include/Acts/EventData/MultiTrajectory.hpp 71.25% <ø> (ø)
Core/include/Acts/EventData/TrackContainer.hpp 88.73% <ø> (ø)
Core/include/Acts/EventData/TrackProxy.hpp 84.80% <ø> (ø)
...e/include/Acts/EventData/VectorMultiTrajectory.hpp 61.13% <ø> (ø)
...re/include/Acts/EventData/VectorTrackContainer.hpp 51.00% <ø> (ø)

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

@paulgessinger paulgessinger removed the 🛑 blocked This item is blocked by another item label May 23, 2023
@paulgessinger paulgessinger requested a review from andiwand May 23, 2023 13:51
@github-actions
Copy link

github-actions bot commented May 23, 2023

📊 Physics performance monitoring for c85abba

Summary
Full report
Seeding: seeded, truth estimated, orthogonal
CKF: seeded, truth smeared, truth estimated, orthogonal
IVF: seeded, truth smeared, truth estimated, orthogonal
AMVF: seeded, truth smeared, truth estimated, orthogonal
Ambiguity resolution: seeded, orthogonal
Truth tracking
Truth tracking (GSF)

Vertexing

Vertexing vs. mu
IVF seeded

IVF truth_smeared

IVF truth_estimated

IVF orthogonal

AMVF seeded

AMVF truth_smeared

AMVF truth_estimated

AMVF orthogonal

Seeding

Seeding seeded

Seeding truth_estimated

Seeding orthogonal

CKF

CKF seeded

CKF truth_smeared

CKF truth_estimated

CKF orthogonal

Ambiguity resolution

seeded

Truth tracking (Kalman Filter)

Truth tracking

Truth tracking (GSF)

Truth tracking

Copy link
Member

@benjaminhuth benjaminhuth left a comment

Choose a reason for hiding this comment

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

Looks good!

@kodiakhq kodiakhq bot merged commit 20a45ed into acts-project:main May 24, 2023
@paulgessinger paulgessinger modified the milestones: next, v27.0.0 Jun 20, 2023
kodiakhq bot pushed a commit that referenced this pull request Oct 23, 2023
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:
- #2093 
- #2092 
- #2091 
- #2090
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants