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!: Fitted params to fitted momenta #2402

Conversation

felix-russo
Copy link
Contributor

@felix-russo felix-russo commented Aug 25, 2023

reopening #2359

The output of a vertex fitter is

  • a vertex position
  • the track momenta at this position

Previously, the track momenta from the vertex fit were saved as GenericTrackParameters, where we set d0 = z0 = 0. For the corresponding covariance matrices, we partly used incorrect values (one cannot, in fact, compute entries like Cov(d0, phi) or Cov(z0, z0) as d0 and z0 are not part of the vertex fitting model).

In this PR, fittedParams are replaced by fittedMomenta (i.e., a 3 vector, its covariance matrix, and its cross-covariance with the vertex). My aim is

  • to make the way the vertex fitter works more obvious and
  • to remove the incorrect covariances.

felix-russo and others added 24 commits August 10, 2023 10:50
This reverts commit 59a494b.
…sso/acts into fitted-params-to-fitted-momenta
…sso/acts into fitted-params-to-fitted-momenta
@github-actions github-actions bot added Component - Core Affects the Core module Infrastructure Changes to build tools, continous integration, ... Component - Examples Affects the Examples module Vertexing Changes Performance labels Aug 25, 2023
@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

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

Comparison is base (c17a685) 49.63% compared to head (25909ed) 49.69%.
Report is 107 commits behind head on main.

❗ Current head 25909ed differs from pull request most recent head 5205894. Consider uploading reports for the commit 5205894 to get more accurate results

Files Patch % Lines
...nclude/Acts/Vertexing/KalmanVertexTrackUpdater.ipp 15.38% 0 Missing and 11 partials ⚠️
...include/Acts/Vertexing/FullBilloirVertexFitter.ipp 22.22% 0 Missing and 7 partials ⚠️
...clude/Acts/Vertexing/AdaptiveMultiVertexFinder.ipp 50.00% 0 Missing and 3 partials ⚠️
...e/include/Acts/Vertexing/IterativeVertexFinder.ipp 33.33% 0 Missing and 2 partials ⚠️
...clude/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp 66.66% 0 Missing and 1 partial ⚠️
...ore/include/Acts/Vertexing/KalmanVertexUpdater.ipp 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2402      +/-   ##
==========================================
+ Coverage   49.63%   49.69%   +0.05%     
==========================================
  Files         474      471       -3     
  Lines       26941    26641     -300     
  Branches    12415    12233     -182     
==========================================
- Hits        13372    13238     -134     
  Misses       4746     4746              
+ Partials     8823     8657     -166     

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

@felix-russo felix-russo marked this pull request as ready for review August 29, 2023 14:44
@paulgessinger paulgessinger modified the milestones: v30.0.0, v31.0.0 Sep 18, 2023
@paulgessinger paulgessinger modified the milestones: v31.0.0, v32.0.0 Nov 15, 2023
@andiwand
Copy link
Contributor

can you resolve the conflicts @felix-russo ?

@paulgessinger
Copy link
Member

@andiwand I think this became a much wider discussion in the context of ATLAS integration.

@felix-russo
Copy link
Contributor Author

felix-russo commented Nov 20, 2023

Conflicts are resolved

Before I continue working on this (which means preparing a parallel PR in Athena if we stick with our current plan for the ATLAS integration) I would like to get #2544 in (which also needs some more work)

@felix-russo felix-russo marked this pull request as draft November 20, 2023 15:05
@github-actions github-actions bot added Stale and removed Stale labels Dec 20, 2023
@felix-russo
Copy link
Contributor Author

Closing this due to time constraints 😢

@felix-russo felix-russo deleted the fitted-params-to-fitted-momenta branch January 20, 2024 13:44
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.

3 participants