feat: Dyn col copying btw. different backend (Track+TrackState) #2712

paulgessinger · 2023-11-22T10:33:45Z

This PR adds:

Support for MultiTrajectory to copy Dynamic Columns at all (not implemented before)
TrackContainer to copy dynamic columns between backends, by refactoring the way the columns are looked up.
- This requires changing the backend contract to add a method dynamicKeys_impl to get hashes of all the dynamic column keys + changing the copyDynamicColumns_impl method.

codecov · 2023-11-22T11:12:21Z

Codecov Report

Attention: 44 lines in your changes are missing coverage. Please review.

Comparison is base (636eff4) 48.62% compared to head (5ce1555) 48.62%.

Files	Patch %	Lines
...ts/EventData/detail/MultiTrajectoryTestsCommon.hpp	24.24%	0 Missing and 25 partials ⚠️
Core/src/EventData/VectorTrackContainer.cpp	16.66%	2 Missing and 3 partials ⚠️
Core/include/Acts/EventData/TrackContainer.hpp	25.00%	0 Missing and 3 partials ⚠️
Core/src/EventData/VectorMultiTrajectory.cpp	57.14%	0 Missing and 3 partials ⚠️
Core/include/Acts/EventData/MultiTrajectory.hpp	71.42%	0 Missing and 2 partials ⚠️
...e/include/Acts/EventData/VectorMultiTrajectory.hpp	50.00%	0 Missing and 2 partials ⚠️
...re/include/Acts/EventData/VectorTrackContainer.hpp	66.66%	0 Missing and 2 partials ⚠️
...re/include/Acts/EventData/detail/DynamicColumn.hpp	80.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2712      +/-   ##
==========================================
- Coverage   48.62%   48.62%   -0.01%     
==========================================
  Files         479      480       +1     
  Lines       27942    28019      +77     
  Branches    13213    13254      +41     
==========================================
+ Hits        13588    13625      +37     
- Misses       4793     4802       +9     
- Partials     9561     9592      +31

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hypothesis still)

github-actions · 2023-11-22T14:17:57Z

📊: Physics performance monitoring for `5ce1555`

Full contents

physmon summary

paulgessinger · 2023-11-22T14:22:44Z

Depending on if the backend contract is part of the public API, this might also be breaking.

Core/include/Acts/EventData/VectorTrackContainer.hpp

Added construction from mutable to const track state container (albeit fragile)

This reverts commit 4bbe4af.

This reverts commit 6dcf392.

…py-dynamic-backends

andiwand

looks good from my side 👍

Core/include/Acts/EventData/MultiTrajectory.hpp

tboldagh

Approving, my comments are realy questions rather than something for debate.

Core/include/Acts/EventData/MultiTrajectoryBackendConcept.hpp

Core/include/Acts/EventData/TrackProxy.hpp

Core/include/Acts/EventData/VectorMultiTrajectory.hpp

Core/src/EventData/VectorMultiTrajectory.cpp

Invalidated by push of 3e4873c

tboldagh

No more comments from me.

acts-project-service · 2023-12-13T08:08:34Z

🔴 Athena integration test results [`407df93`]

Build job with this PR failed!

Please investigate the build job for the pipeline!

This moves TrackStateProxy into its own header file, and moves it out of the `detail_lt` namespace. Blocked by: - #2712 BREAKING CHANGE: `Acts::detail_lt::TrackStateProxy` moves to `Acts::TrackStateProxy` and include `Acts/EventData/TrackStateProxy.hpp`

This was an inconsistency due to different parallel developments. Blocked by: - #2712

…-project#2712) This PR adds: - Support for MultiTrajectory to copy Dynamic Columns at all (not implemented before) - TrackContainer to copy dynamic columns between backends, by refactoring the way the columns are looked up. - This requires changing the backend contract to add a method `dynamicKeys_impl` to get hashes of all the dynamic column keys + changing the `copyDynamicColumns_impl` method.

…t#2820) This was an inconsistency due to different parallel developments. Blocked by: - acts-project#2712

This is pulled out of #2797 and includes only the doxygen comment changes. Blocked by: - #2712 - #2817 - #2807

…-project#2712) This PR adds: - Support for MultiTrajectory to copy Dynamic Columns at all (not implemented before) - TrackContainer to copy dynamic columns between backends, by refactoring the way the columns are looked up. - This requires changing the backend contract to add a method `dynamicKeys_impl` to get hashes of all the dynamic column keys + changing the `copyDynamicColumns_impl` method.

…t#2807) This moves TrackStateProxy into its own header file, and moves it out of the `detail_lt` namespace. Blocked by: - acts-project#2712 BREAKING CHANGE: `Acts::detail_lt::TrackStateProxy` moves to `Acts::TrackStateProxy` and include `Acts/EventData/TrackStateProxy.hpp`

…t#2820) This was an inconsistency due to different parallel developments. Blocked by: - acts-project#2712

This is pulled out of acts-project#2797 and includes only the doxygen comment changes. Blocked by: - acts-project#2712 - acts-project#2817 - acts-project#2807

feat: Dyn col copying btw. different backend (Track+TrackState)

8956d6c

paulgessinger added this to the next milestone Nov 22, 2023

paulgessinger requested a review from tboldagh November 22, 2023 10:33

github-actions bot added Component - Core Affects the Core module Event Data Model labels Nov 22, 2023

paulgessinger added 2 commits November 22, 2023 14:11

update PODIO track backend to be complete and current (missing particle

bfb8f41

hypothesis still)

Add particle hypo support in PODIO backend

e5dd9fb

github-actions bot added the Component - Plugins Affects one or more Plugins label Nov 22, 2023

missing include

0bcaf7f

paulgessinger commented Nov 22, 2023

View reviewed changes

Core/include/Acts/EventData/VectorTrackContainer.hpp Outdated Show resolved Hide resolved

paulgessinger and others added 9 commits November 29, 2023 16:02

Merge branch 'main' into refactor/track-edm-copy-dynamic-backends

a694752

complete actual copy implementation

32d9918

build: Add target to build all unit tests

4bbe4af

fix track container test

0a7506f

dynamic column test implementation working

edfa27f

Added construction from mutable to const track state container (albeit fragile)

centralize dynamic columns recovery and use typelist

6dcf392

Revert "build: Add target to build all unit tests"

4ddf5c3

This reverts commit 4bbe4af.

Revert "centralize dynamic columns recovery and use typelist"

e6b50de

This reverts commit 6dcf392.

Merge branch 'main' into refactor/track-edm-copy-dynamic-backends

3e024c1

paulgessinger marked this pull request as ready for review November 29, 2023 17:59

paulgessinger added 3 commits December 4, 2023 16:37

fix concepts

c9df58e

fix another concept violation

620da1d

Merge remote-tracking branch 'origin/main' into refactor/track-edm-co…

bb039f9

…py-dynamic-backends

andiwand previously approved these changes Dec 11, 2023

View reviewed changes

tboldagh reviewed Dec 11, 2023

View reviewed changes

Core/include/Acts/EventData/MultiTrajectory.hpp Outdated Show resolved Hide resolved

Core/include/Acts/EventData/MultiTrajectory.hpp Show resolved Hide resolved

paulgessinger mentioned this pull request Dec 11, 2023

refactor!: Split TrackStateProxy into own header, public #2807

Merged

remove redundantly added container getters

14ccf93

paulgessinger dismissed andiwand’s stale review via ceb3604 December 12, 2023 12:43

add a typename

f98a245

tboldagh previously approved these changes Dec 12, 2023

View reviewed changes

use std::forward_iterator as concept instead of own

3e4873c

andiwand approved these changes Dec 12, 2023

View reviewed changes

andiwand added the automerge label Dec 12, 2023

paulgessinger mentioned this pull request Dec 12, 2023

docs: Track EDM doxygen comment updates #2816

Merged

tboldagh approved these changes Dec 12, 2023

View reviewed changes

paulgessinger mentioned this pull request Dec 12, 2023

refactor: PODIO backends don't need to fill dynamic keys #2820

Merged

Merge branch 'main' into refactor/track-edm-copy-dynamic-backends

5ce1555

kodiakhq bot merged commit 407df93 into acts-project:main Dec 13, 2023
52 checks passed

github-actions bot removed the automerge label Dec 13, 2023

paulgessinger deleted the refactor/track-edm-copy-dynamic-backends branch December 13, 2023 08:05

acts-project-service added the Breaks Athena build This PR breaks the Athena build label Dec 13, 2023

kodiakhq bot pushed a commit that referenced this pull request Dec 14, 2023

refactor: PODIO backends don't need to fill dynamic keys (#2820)

66c671c

This was an inconsistency due to different parallel developments. Blocked by: - #2712

paulgessinger added a commit to paulgessinger/acts that referenced this pull request Dec 15, 2023

refactor: PODIO backends don't need to fill dynamic keys (acts-projec…

7c38bdd

…t#2820) This was an inconsistency due to different parallel developments. Blocked by: - acts-project#2712

kodiakhq bot pushed a commit that referenced this pull request Dec 18, 2023

docs: Track EDM doxygen comment updates (#2816)

ba0ab93

This is pulled out of #2797 and includes only the doxygen comment changes. Blocked by: - #2712 - #2817 - #2807

paulgessinger mentioned this pull request Dec 31, 2023

refactor: Use common direction transform Jacobian #2782

Merged

paulgessinger modified the milestones: next, v32.0.0 Jan 19, 2024

paulgessinger added Breaks Athena build This PR breaks the Athena build and removed Breaks Athena build This PR breaks the Athena build labels Jan 23, 2024

LaraCalic pushed a commit to LaraCalic/acts that referenced this pull request Feb 10, 2024

refactor: PODIO backends don't need to fill dynamic keys (acts-projec…

afbca33

…t#2820) This was an inconsistency due to different parallel developments. Blocked by: - acts-project#2712

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Dyn col copying btw. different backend (Track+TrackState) #2712

feat: Dyn col copying btw. different backend (Track+TrackState) #2712

paulgessinger commented Nov 22, 2023

codecov bot commented Nov 22, 2023 •

edited

Loading

github-actions bot commented Nov 22, 2023 •

edited

Loading

paulgessinger commented Nov 22, 2023

andiwand left a comment

tboldagh left a comment

tboldagh left a comment

acts-project-service commented Dec 13, 2023

feat: Dyn col copying btw. different backend (Track+TrackState) #2712

feat: Dyn col copying btw. different backend (Track+TrackState) #2712

Conversation

paulgessinger commented Nov 22, 2023

codecov bot commented Nov 22, 2023 • edited Loading

Codecov Report

github-actions bot commented Nov 22, 2023 • edited Loading

📊: Physics performance monitoring for 5ce1555

physmon summary

paulgessinger commented Nov 22, 2023

andiwand left a comment

Choose a reason for hiding this comment

tboldagh left a comment

Choose a reason for hiding this comment

tboldagh left a comment

Choose a reason for hiding this comment

acts-project-service commented Dec 13, 2023

🔴 Athena integration test results [407df93]

Build job with this PR failed!

codecov bot commented Nov 22, 2023 •

edited

Loading

github-actions bot commented Nov 22, 2023 •

edited

Loading

📊: Physics performance monitoring for `5ce1555`

🔴 Athena integration test results [`407df93`]