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 direct navigator initialization #3183

Merged

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented May 10, 2024

After #3181 and #3182 we can properly initialize the DirectNavigator with the surface sequence without a special Actor modifying the sequence at the right moment. This is directly applied for the KF and GSF.

blocked by:

@andiwand andiwand added this to the v36.0.0 milestone May 10, 2024
@github-actions github-actions bot added Component - Core Affects the Core module Component - Fatras Affects the Fatras module Component - Examples Affects the Examples module Component - Documentation Affects the documentation Vertexing Track Finding Track Fitting labels May 10, 2024
@andiwand andiwand marked this pull request as ready for review May 10, 2024 12:16
@andiwand andiwand marked this pull request as draft May 15, 2024 06:57
@andiwand andiwand added the 🛑 blocked This item is blocked by another item label May 15, 2024
@andiwand andiwand force-pushed the refactor-direct-navigator-initialization branch from 7bfd0f0 to 522053f Compare June 8, 2024 15:48
Copy link

codecov bot commented Jun 8, 2024

Codecov Report

Attention: Patch coverage is 68.48485% with 52 lines in your changes missing coverage. Please review.

Project coverage is 47.66%. Comparing base (75ef913) to head (cabfe79).
Report is 77 commits behind head on main.

Files Patch % Lines
Core/include/Acts/Propagator/DirectNavigator.hpp 65.00% 4 Missing and 17 partials ⚠️
...lude/Acts/Propagator/DenseEnvironmentExtension.hpp 0.00% 0 Missing and 6 partials ⚠️
...de/Acts/TrackFinding/CombinatorialKalmanFilter.hpp 0.00% 4 Missing and 1 partial ⚠️
Core/include/Acts/TrackFitting/KalmanFitter.hpp 54.54% 0 Missing and 5 partials ⚠️
Core/include/Acts/Propagator/PropagatorOptions.hpp 77.77% 0 Missing and 4 partials ⚠️
Core/include/Acts/Propagator/PropagatorState.hpp 40.00% 0 Missing and 3 partials ⚠️
Core/include/Acts/Propagator/EigenStepper.ipp 50.00% 2 Missing ⚠️
Core/include/Acts/Propagator/AtlasStepper.hpp 50.00% 0 Missing and 1 partial ⚠️
Core/include/Acts/Propagator/Navigator.hpp 88.88% 0 Missing and 1 partial ⚠️
Core/include/Acts/Propagator/TryAllNavigator.hpp 85.71% 0 Missing and 1 partial ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3183      +/-   ##
==========================================
- Coverage   47.66%   47.66%   -0.01%     
==========================================
  Files         509      511       +2     
  Lines       29425    29421       -4     
  Branches    14131    14131              
==========================================
- Hits        14026    14023       -3     
+ Misses       5285     5281       -4     
- Partials    10114    10117       +3     

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

@andiwand andiwand force-pushed the refactor-direct-navigator-initialization branch from 522053f to cabfe79 Compare June 8, 2024 17:35
@andiwand andiwand marked this pull request as ready for review June 8, 2024 17:35
@github-actions github-actions bot added Stale and removed Stale labels Jul 8, 2024
@andiwand andiwand force-pushed the refactor-direct-navigator-initialization branch from cabfe79 to 9ce6967 Compare July 12, 2024 19:52
@github-actions github-actions bot added Component - Documentation Affects the documentation and removed Component - Fatras Affects the Fatras module Component - Examples Affects the Examples module Component - Documentation Affects the documentation Vertexing labels Jul 12, 2024
kodiakhq bot pushed a commit that referenced this pull request Jul 15, 2024
Move `startSurface` and `targetSurface` to `Navigator::Options`. In two followup PRs (#3189, #3183) the options will be extended by direct navigator surface and external surfaces.

blocked by:
- #3181
- #3190
@andiwand andiwand force-pushed the refactor-direct-navigator-initialization branch from 95a67da to ee987f4 Compare July 15, 2024 20:49
@andiwand andiwand removed the 🛑 blocked This item is blocked by another item label Jul 15, 2024
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.

Looks good to me. I like that we can get rid of the initialization actor.

Can you add a few words on the intention of this change to the PR description (also for future reference)

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.

Looks good to me. I like that we can get rid of the initialization actor this way.

Can you add a few words describing the change to the PR description?

Copy link

@kodiakhq kodiakhq bot merged commit 918b464 into acts-project:main Jul 16, 2024
44 checks passed
@andiwand andiwand deleted the refactor-direct-navigator-initialization branch July 16, 2024 17:41
@acts-project-service
Copy link
Collaborator

🔴 Athena integration test results [918b464]

Build job with this PR failed!

Please investigate the build job for the pipeline!

@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaks Athena build This PR breaks the Athena build Component - Core Affects the Core module Component - Documentation Affects the documentation Track Fitting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants