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: Move large parts of Vertexing to .cpp files #2953

Merged

Conversation

paulgessinger
Copy link
Member

@paulgessinger paulgessinger commented Feb 15, 2024

This allows separate compilation, and should reduce memory footprint while compiling (as documented in #2842)
They are:

  • AdaptiveMultiVertexFitter
  • IterativeVertexFitter
  • AdaptiveGridDensityVertexFinder
  • ZScanVertexFinder
  • HelicalTrackLinearizer
  • FullBilloirVertexFitter
  • AdaptiveMultiVertexFinder
  • Vertex
  • GaussianTrackDensity
  • NumericalTrackLinearizer

Part of:

Blocked by:

@paulgessinger paulgessinger added the 🛑 blocked This item is blocked by another item label Feb 15, 2024
@paulgessinger paulgessinger added this to the next milestone Feb 15, 2024
@paulgessinger paulgessinger force-pushed the refactor/vtx-templates-part10-cpp-files branch from 4d08ceb to 80e812a Compare February 15, 2024 14:41
@github-actions github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Vertexing labels Feb 15, 2024
@paulgessinger paulgessinger force-pushed the refactor/vtx-templates-part10-cpp-files branch 2 times, most recently from af35c8d to 774d1c5 Compare February 15, 2024 15:04
Copy link

codecov bot commented Feb 15, 2024

Codecov Report

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

Comparison is base (60cfa75) 48.78% compared to head (51205fc) 48.74%.

Files Patch % Lines
Core/src/Vertexing/Vertex.cpp 73.84% 9 Missing and 8 partials ⚠️
Core/src/Vertexing/IterativeVertexFinder.cpp 47.82% 8 Missing and 4 partials ⚠️
Core/src/Vertexing/ZScanVertexFinder.cpp 50.00% 2 Missing and 2 partials ⚠️
Core/src/Vertexing/FullBilloirVertexFitter.cpp 60.00% 0 Missing and 2 partials ⚠️
Core/src/Vertexing/AdaptiveMultiVertexFitter.cpp 88.88% 1 Missing ⚠️
Core/src/Vertexing/GaussianTrackDensity.cpp 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2953      +/-   ##
==========================================
- Coverage   48.78%   48.74%   -0.05%     
==========================================
  Files         493      493              
  Lines       28905    28918      +13     
  Branches    13747    13760      +13     
==========================================
- Hits        14102    14096       -6     
- Misses       4911     4921      +10     
- Partials     9892     9901       +9     

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

@paulgessinger paulgessinger force-pushed the refactor/vtx-templates-part10-cpp-files branch 8 times, most recently from 1616206 to 45f3dfa Compare February 21, 2024 10:13
@paulgessinger paulgessinger removed the 🛑 blocked This item is blocked by another item label Feb 21, 2024
@github-actions github-actions bot removed the Component - Examples Affects the Examples module label Feb 21, 2024
@paulgessinger paulgessinger marked this pull request as ready for review February 21, 2024 10:13
andiwand
andiwand previously approved these changes Feb 21, 2024
Core/src/Vertexing/ZScanVertexFinder.cpp Outdated Show resolved Hide resolved
Core/src/Vertexing/TrackDensityVertexFinder.cpp Outdated Show resolved Hide resolved
@paulgessinger
Copy link
Member Author

I removed the trailing return types now @andiwand

andiwand
andiwand previously approved these changes Feb 21, 2024
@kodiakhq kodiakhq bot merged commit e9ab1f7 into acts-project:main Feb 21, 2024
56 checks passed
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Feb 21, 2024
kodiakhq bot pushed a commit that referenced this pull request Feb 27, 2024
This PR moves the code of the `ImpactPointEstimator` from the header into a compiled file. To allow this, I have to make some changes to the interface to remove templates.

In the process, I introduced `Eigen::Map<ActsDynamicVector>` as an argument to `getDistanceAndMomentum` and `getVertexCompatibility`.
Part of:
- #2842 

Blocked by:
- #2953
@paulgessinger paulgessinger deleted the refactor/vtx-templates-part10-cpp-files branch February 29, 2024 22:23
@paulgessinger paulgessinger modified the milestones: next, v33.0.0 Mar 6, 2024
@paulgessinger paulgessinger removed the Fails Athena tests This PR causes a failure in the Athena tests label Mar 6, 2024
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
…#2953)

This allows separate compilation, and should reduce memory footprint while compiling (as documented in acts-project#2842)
They are:

- `AdaptiveMultiVertexFitter`
- `IterativeVertexFitter`
- `AdaptiveGridDensityVertexFinder`
- `ZScanVertexFinder`
- `HelicalTrackLinearizer`
- `FullBilloirVertexFitter`
- `AdaptiveMultiVertexFinder`
- `Vertex`
- `GaussianTrackDensity`
- `NumericalTrackLinearizer`

Part of:
- acts-project#2842

Blocked by:
- acts-project#2952
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
This PR moves the code of the `ImpactPointEstimator` from the header into a compiled file. To allow this, I have to make some changes to the interface to remove templates.

In the process, I introduced `Eigen::Map<ActsDynamicVector>` as an argument to `getDistanceAndMomentum` and `getVertexCompatibility`.
Part of:
- acts-project#2842 

Blocked by:
- acts-project#2953
asalzburger pushed a commit to asalzburger/acts that referenced this pull request May 21, 2024
…#2953)

This allows separate compilation, and should reduce memory footprint while compiling (as documented in acts-project#2842)
They are:

- `AdaptiveMultiVertexFitter`
- `IterativeVertexFitter`
- `AdaptiveGridDensityVertexFinder`
- `ZScanVertexFinder`
- `HelicalTrackLinearizer`
- `FullBilloirVertexFitter`
- `AdaptiveMultiVertexFinder`
- `Vertex`
- `GaussianTrackDensity`
- `NumericalTrackLinearizer`

Part of:
- acts-project#2842

Blocked by:
- acts-project#2952
asalzburger pushed a commit to asalzburger/acts that referenced this pull request May 21, 2024
This PR moves the code of the `ImpactPointEstimator` from the header into a compiled file. To allow this, I have to make some changes to the interface to remove templates.

In the process, I introduced `Eigen::Map<ActsDynamicVector>` as an argument to `getDistanceAndMomentum` and `getVertexCompatibility`.
Part of:
- acts-project#2842 

Blocked by:
- acts-project#2953
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 Vertexing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants