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: use time info in Kalman vertex fit #2544

Closed

Conversation

felix-russo
Copy link
Contributor

@felix-russo felix-russo commented Oct 14, 2023

Since we introduced the time coordinate in tracking, I propose to the same in the Kalman vertex fitter now that the Jacobians of HelicalTrackLinearizer are filled entirely.

Starting to factor this out, blocked by:

@github-actions github-actions bot added Component - Core Affects the Core module Vertexing labels Oct 14, 2023
@codecov
Copy link

codecov bot commented Oct 15, 2023

Codecov Report

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

Comparison is base (37a2f9e) 49.54% compared to head (989cebd) 49.61%.
Report is 1 commits behind head on main.

Files Patch % Lines
...ore/include/Acts/Vertexing/KalmanVertexUpdater.ipp 26.21% 4 Missing and 72 partials ⚠️
...clude/Acts/Vertexing/AdaptiveMultiVertexFinder.ipp 0.00% 4 Missing ⚠️
...nclude/Acts/Vertexing/TrackDensityVertexFinder.ipp 0.00% 0 Missing and 3 partials ⚠️
Core/include/Acts/TrackFitting/KalmanFitter.hpp 33.33% 0 Missing and 2 partials ⚠️
...clude/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2544      +/-   ##
==========================================
+ Coverage   49.54%   49.61%   +0.06%     
==========================================
  Files         474      473       -1     
  Lines       26929    26922       -7     
  Branches    12425    12405      -20     
==========================================
+ Hits        13343    13357      +14     
- Misses       4749     4753       +4     
+ Partials     8837     8812      -25     

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

@github-actions
Copy link

github-actions bot commented Oct 15, 2023

📊: Physics performance monitoring for 989cebd

Full contents
🟥 summary not found!

@felix-russo
Copy link
Contributor Author

Encountered some changes in physmon, the PR seems to improve performance for AMVF orthogonal

image

but deteriorate performance for AMVF (+grid seeder) seeded

image

@paulgessinger paulgessinger added this to the next milestone Oct 23, 2023
@felix-russo felix-russo changed the title refactor: always use time info in Kalman Fitter Vertexing refactor: always use time info in Kalman vertex fit Oct 23, 2023
andiwand
andiwand previously approved these changes Oct 23, 2023
Copy link
Contributor

@andiwand andiwand 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 think the performance needs to be studied in a more global setting anyway. I am fine with a small loss

@github-actions github-actions bot added the Infrastructure Changes to build tools, continous integration, ... label Oct 24, 2023
@felix-russo felix-russo changed the title refactor: always use time info in Kalman vertex fit refactor: use time info in Kalman vertex fit Oct 24, 2023
@felix-russo felix-russo marked this pull request as ready for review October 24, 2023 08:58
@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Oct 25, 2023
@paulgessinger paulgessinger modified the milestones: v31.0.0, next Nov 17, 2023
@andiwand
Copy link
Contributor

can you resolve the conflict @felix-russo ?

@github-actions github-actions bot added the Component - Examples Affects the Examples module label Nov 21, 2023
@felix-russo
Copy link
Contributor Author

Closing in favor of #2751

@felix-russo felix-russo deleted the always-d-time-fitting branch December 1, 2023 16:15
@paulgessinger paulgessinger modified the milestones: next, v31.2.0 Dec 7, 2023
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 Component - Examples Affects the Examples module 👷‍♀️ User Action Needed Fails Athena tests This PR causes a failure in the Athena tests Infrastructure Changes to build tools, continous integration, ... Vertexing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants