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: Manual Propagator initialization and AnySurfaceReached aborter #3208

Merged
merged 12 commits into from
May 30, 2024

Conversation

andiwand
Copy link
Contributor

This allows to manually initialize the propagation so propagate(state) can be called multiple times without destruction of the state in-between. I also added an AnySurfaceReached aborter which, in combination with the other change, allows users to jump from surface to surface using the same propagator state without any resets.

@andiwand andiwand added this to the next milestone May 22, 2024
@github-actions github-actions bot added the Component - Core Affects the Core module label May 22, 2024
@andiwand andiwand changed the title feat: Manual Propagator initialization feat: Manual Propagator initialization and AnySurfaceReached aborter May 22, 2024
@andiwand andiwand added the 🚧 WIP Work-in-progress label May 23, 2024
@andiwand andiwand removed the 🚧 WIP Work-in-progress label May 23, 2024
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.

Some comments

Core/include/Acts/Propagator/Propagator.hpp Show resolved Hide resolved
Core/include/Acts/Propagator/Propagator.hpp Outdated Show resolved Hide resolved
Copy link

codecov bot commented May 27, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 47.59%. Comparing base (d13dc2b) to head (742743a).
Report is 284 commits behind head on main.

Files with missing lines Patch % Lines
Core/include/Acts/Propagator/Propagator.ipp 85.71% 0 Missing and 1 partial ⚠️
Core/include/Acts/TrackFitting/KalmanFitter.hpp 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3208   +/-   ##
=======================================
  Coverage   47.59%   47.59%           
=======================================
  Files         507      507           
  Lines       29135    29137    +2     
  Branches    13976    13976           
=======================================
+ Hits        13866    13869    +3     
- Misses       5268     5269    +1     
+ Partials    10001     9999    -2     

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

@andiwand andiwand requested a review from benjaminhuth May 27, 2024 15:54
benjaminhuth
benjaminhuth previously approved these changes May 27, 2024
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.

lgtm, I do not yet start merging to wait for @ssdetlab approval

@ssdetlab
Copy link
Contributor

Hi @andiwand, @benjaminhuth. The changes look reasonable to me. Maybe add a small comment clarifying that the makeState call is going to initialize the navigator, but I don't insist.

@andiwand andiwand requested a review from benjaminhuth May 29, 2024 13:40
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.

Let's get it in!

@kodiakhq kodiakhq bot merged commit a48fa8a into acts-project:main May 30, 2024
55 checks passed
@andiwand andiwand deleted the propagator-initialize branch May 30, 2024 06:50
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label May 30, 2024
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 31, 2024
…ter (acts-project#3208)

This allows to manually initialize the propagation so `propagate(state)` can be called multiple times without destruction of the state in-between. I also added an `AnySurfaceReached` aborter which, in combination with the other change, allows users to jump from surface to surface using the same propagator state without any resets.
@andiwand andiwand modified the milestones: next, v35.1.0 Jun 1, 2024
Matthewharri pushed a commit to Matthewharri/acts that referenced this pull request Jun 18, 2024
…ter (acts-project#3208)

This allows to manually initialize the propagation so `propagate(state)` can be called multiple times without destruction of the state in-between. I also added an `AnySurfaceReached` aborter which, in combination with the other change, allows users to jump from surface to surface using the same propagator state without any resets.
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 Fails Athena tests This PR causes a failure in the Athena tests Track Fitting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants