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: Refactor SurfaceReached aborter #2603

Merged
merged 14 commits into from
Nov 20, 2023

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Oct 31, 2023

With these changes I try to decouple the navigator and surface reached aborter further. I don't believe that the navigator has to care about the target surface other than optimization reasons while the surface reached aborter should only care about the surface it is supposed to care about without looking at or touching the navigation state.

blocked by

@andiwand andiwand added this to the next milestone Oct 31, 2023
@andiwand andiwand changed the title refactor Refactor SurfaceReached aborter refactor: Refactor SurfaceReached aborter Oct 31, 2023
Copy link

codecov bot commented Nov 1, 2023

Codecov Report

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

Comparison is base (27fc8ec) 49.64% compared to head (ad11992) 49.56%.

❗ Current head ad11992 differs from pull request most recent head 4770a64. Consider uploading reports for the commit 4770a64 to get more accurate results

Files Patch % Lines
...e/include/Acts/Propagator/MultiStepperAborters.hpp 38.88% 4 Missing and 7 partials ⚠️
Core/include/Acts/Propagator/StandardAborters.hpp 40.00% 0 Missing and 9 partials ⚠️
...de/Acts/TrackFinding/CombinatorialKalmanFilter.hpp 45.45% 0 Missing and 6 partials ⚠️
Core/include/Acts/TrackFitting/KalmanFitter.hpp 75.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2603      +/-   ##
==========================================
- Coverage   49.64%   49.56%   -0.08%     
==========================================
  Files         474      474              
  Lines       26946    26923      -23     
  Branches    12415    12422       +7     
==========================================
- Hits        13376    13344      -32     
- Misses       4747     4750       +3     
- Partials     8823     8829       +6     

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

@andiwand andiwand added the 🚧 WIP Work-in-progress label Nov 2, 2023
@andiwand andiwand added 🛑 blocked This item is blocked by another item and removed 🚧 WIP Work-in-progress labels Nov 3, 2023
@andiwand
Copy link
Contributor Author

andiwand commented Nov 3, 2023

there is some physmon diff for the gsf but it looks marginal to me. maybe @benjaminhuth can have a look. will not update the reference for now

@benjaminhuth
Copy link
Member

Seems to improve things actually, right?

Maybe because the overstep limit is now properly propagated?

kodiakhq bot pushed a commit that referenced this pull request Nov 7, 2023
Currently it is not possible to discover in which stage of propagation we are in the actors and aborters. By introducing a propagator stage enum, adding it to the state and setting it in the proagator loop this can be used in the actors and aborters.

Pulled out of #2603
@andiwand andiwand removed the 🛑 blocked This item is blocked by another item label Nov 8, 2023
@paulgessinger
Copy link
Member

This does not cause any output changes in Athena.

Copy link
Member

@paulgessinger paulgessinger left a comment

Choose a reason for hiding this comment

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

I think i like this.

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 from my side! (mainly looked at the multi stepper aborter)

@github-actions github-actions bot added Infrastructure Changes to build tools, continous integration, ... Component - Examples Affects the Examples module Changes Performance labels Nov 17, 2023
@kodiakhq kodiakhq bot merged commit eae867a into acts-project:main Nov 20, 2023
50 checks passed
@andiwand andiwand deleted the refactor-surface-reached-aborter branch November 20, 2023 12:03
@acts-project-service
Copy link
Collaborator

kodiakhq bot pushed a commit that referenced this pull request Nov 20, 2023
Aimed to improve consistency and split surface and volume interactions into two logical parts

pulled out of #2603
@paulgessinger paulgessinger modified the milestones: next, v31.1.0 Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes Performance Component - Core Affects the Core module Component - Examples Affects the Examples module Infrastructure Changes to build tools, continous integration, ... Track Finding Track Fitting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants