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: Consistent surface tolerance for propagation #2292

Merged

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Jul 10, 2023

In #2142 I discovered that the ridders propagator depends highly on the step size taken. At the same time we still use the global static surface tolerance in most of the places which I try to improve here. This allows us to consistently set the surface tolerance for the propagation and to improve the accuracy of the ridders propagator.

Includes a bug fix for AtlasStepper I discovered after making these changes. The stepper ignored the stepping tolerance in backward mode.

@andiwand andiwand added this to the next milestone Jul 10, 2023
@github-actions github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Track Finding Track Fitting labels Jul 10, 2023
@CarloVarni CarloVarni added the 🚧 WIP Work-in-progress label Jul 11, 2023
@github-actions
Copy link

github-actions bot commented Jul 11, 2023

📊 Physics performance monitoring for 2032340

Summary
Full report
Seeding: seeded, truth estimated, orthogonal
CKF: seeded, truth smeared, truth estimated, orthogonal
IVF: seeded, truth smeared, truth estimated, orthogonal
AMVF: seeded, truth smeared, truth estimated, orthogonal
Ambiguity resolution: seeded, orthogonal
Truth tracking
Truth tracking (GSF)

Vertexing

Vertexing vs. mu
IVF seeded

IVF truth_smeared

IVF truth_estimated

IVF orthogonal

AMVF seeded

AMVF truth_smeared

AMVF truth_estimated

AMVF orthogonal

Seeding

Seeding seeded

Seeding truth_estimated

Seeding orthogonal

CKF

CKF seeded

CKF truth_smeared

CKF truth_estimated

CKF orthogonal

Ambiguity resolution

seeded

Truth tracking (Kalman Filter)

Truth tracking

Truth tracking (GSF)

Truth tracking

@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Merging #2292 (5bf16ce) into main (c499dfe) will decrease coverage by 0.01%.
The diff coverage is 17.94%.

❗ Current head 5bf16ce differs from pull request most recent head 2032340. Consider uploading reports for the commit 2032340 to get more accurate results

@@            Coverage Diff             @@
##             main    #2292      +/-   ##
==========================================
- Coverage   49.27%   49.27%   -0.01%     
==========================================
  Files         450      450              
  Lines       25408    25406       -2     
  Branches    11727    11727              
==========================================
- Hits        12521    12519       -2     
  Misses       4549     4549              
  Partials     8338     8338              
Impacted Files Coverage Δ
Core/include/Acts/Propagator/DirectNavigator.hpp 75.00% <0.00%> (ø)
Core/include/Acts/Propagator/EigenStepper.ipp 53.84% <0.00%> (ø)
...e/include/Acts/Propagator/MultiStepperAborters.hpp 65.51% <0.00%> (-1.15%) ⬇️
Core/include/Acts/Propagator/Propagator.ipp 40.25% <ø> (ø)
.../include/Acts/Propagator/detail/SteppingHelper.hpp 53.84% <0.00%> (ø)
...de/Acts/TrackFinding/CombinatorialKalmanFilter.hpp 31.60% <0.00%> (-0.08%) ⬇️
Core/include/Acts/TrackFitting/KalmanFitter.hpp 42.93% <0.00%> (-0.13%) ⬇️
Core/include/Acts/Propagator/AtlasStepper.hpp 72.02% <7.14%> (ø)
Core/include/Acts/Propagator/Navigator.hpp 58.11% <16.66%> (+0.03%) ⬆️
...lude/Acts/Navigation/SurfaceCandidatesUpdators.hpp 67.18% <50.00%> (+0.52%) ⬆️
... and 3 more

... and 1 file with indirect coverage changes

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

@andiwand andiwand marked this pull request as ready for review July 11, 2023 14:26
@andiwand andiwand removed the 🚧 WIP Work-in-progress label Jul 11, 2023
Copy link
Contributor

@asalzburger asalzburger left a comment

Choose a reason for hiding this comment

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

Excellent job, great improvement.

@kodiakhq kodiakhq bot merged commit 93fb63e into acts-project:main Jul 12, 2023
54 of 55 checks passed
@andiwand andiwand deleted the refactor-consistent-surface-tolerance branch July 12, 2023 11:17
@paulgessinger paulgessinger modified the milestones: next, v27.2.0 Jul 24, 2023
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Jul 24, 2023
…2292)

In acts-project#2142 I discovered that the ridders propagator depends highly on the step size taken. At the same time we still use the global static surface tolerance in most of the places which I try to improve here. This allows us to consistently set the surface tolerance for the propagation and to improve the accuracy of the ridders propagator.

Includes a bug fix for `AtlasStepper` I discovered after making these changes. The stepper ignored the stepping tolerance in backward mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants