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 (optional) forward linking of track states #2418

Merged
merged 7 commits into from
Sep 21, 2023

Conversation

paulgessinger
Copy link
Member

@paulgessinger paulgessinger commented Sep 5, 2023

A track can now initiate forward linking of its track states. The track
states have a new property, next that optionally makes the sequence a
doubly linked list. This is not filled by default (not even by
appendTrackState) because for branching cases, it's not unambiguous.

linkForward() on a track will initiate the forward linking and store
the innermost track state index for easy access. reverseTrackStates
also does this, since it's only valid for non-branching trackstate
sequences anyway.

Also adds a innermostTrackState() getter, which returns a track state proxy in case the track is forward linked and has a stemIndex

Blocked by:

@paulgessinger paulgessinger added the 🛑 blocked This item is blocked by another item label Sep 5, 2023
@codecov
Copy link

codecov bot commented Sep 5, 2023

Codecov Report

Merging #2418 (530c91c) into main (5cdea63) will increase coverage by 0.01%.
The diff coverage is 59.25%.

@@            Coverage Diff             @@
##             main    #2418      +/-   ##
==========================================
+ Coverage   49.80%   49.81%   +0.01%     
==========================================
  Files         466      466              
  Lines       26141    26190      +49     
  Branches    11989    12009      +20     
==========================================
+ Hits        13019    13047      +28     
- Misses       4615     4620       +5     
- Partials     8507     8523      +16     
Files Changed Coverage Δ
Core/src/EventData/VectorTrackContainer.cpp 27.27% <20.00%> (-0.39%) ⬇️
...re/include/Acts/EventData/VectorTrackContainer.hpp 50.45% <25.00%> (-0.96%) ⬇️
...e/include/Acts/EventData/VectorMultiTrajectory.hpp 58.79% <33.33%> (-0.36%) ⬇️
Core/include/Acts/EventData/TrackProxy.hpp 77.90% <66.66%> (-1.25%) ⬇️
Core/src/EventData/VectorMultiTrajectory.cpp 71.33% <66.66%> (-0.10%) ⬇️
Core/include/Acts/EventData/MultiTrajectory.hpp 70.55% <68.75%> (-0.20%) ⬇️
Core/include/Acts/EventData/TrackContainer.hpp 88.46% <80.00%> (-0.58%) ⬇️

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

@asalzburger
Copy link
Contributor

Very nice, indeed! @EdwardMoyse you mind reviewing this one?

@paulgessinger paulgessinger added this to the v30.0.0 milestone Sep 6, 2023
kodiakhq bot pushed a commit that referenced this pull request Sep 11, 2023
This was previously still using the `iprevious` member in `IndexData`, but we did have a separate `m_previous` column already that was being ignored.

Preparation for #2418
kodiakhq bot pushed a commit that referenced this pull request Sep 15, 2023
This better indicates that the iterations is outside in

Preparation for #2418.
@paulgessinger paulgessinger removed the 🛑 blocked This item is blocked by another item label Sep 18, 2023
A track can now initiate forward linking of its track states. The track
states have a new property, `next` that optionally makes the sequence a
doubly linked list. This is not filled by default (not even by
`appendTrackState`) because for branching cases, it's not unambiguous.

`linkForward()` on a track will initiate the forward linking and store
the innermost track state index for easy access. `reverseTrackStates`
also does this, since it's only valid for non-branching trackstate
sequences anyway.
@paulgessinger paulgessinger force-pushed the track-state-range-order branch from 7231cc8 to cd2e27e Compare September 18, 2023 12:52
@paulgessinger
Copy link
Member Author

Rebased, conflicts resolved.

@github-actions github-actions bot removed Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins labels Sep 18, 2023
@paulgessinger
Copy link
Member Author

Conflicts resolved.

Copy link
Contributor

@andiwand andiwand 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 66b04e5 into acts-project:main Sep 21, 2023
AJPfleger pushed a commit to AJPfleger/acts that referenced this pull request Sep 29, 2023
A track can now initiate forward linking of its track states. The track
states have a new property, `next` that optionally makes the sequence a
doubly linked list. This is not filled by default (not even by
`appendTrackState`) because for branching cases, it's not unambiguous.

`linkForward()` on a track will initiate the forward linking and store
the innermost track state index for easy access. `reverseTrackStates`
also does this, since it's only valid for non-branching trackstate
sequences anyway.

Also adds a `innermostTrackState()` getter, which returns a track state proxy in case the track is forward linked and has a `stemIndex`

Blocked by:
- acts-project#2425 
- acts-project#2426
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants