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!: Remove CovarianceTransport #2781

Merged
merged 7 commits into from
Dec 16, 2023

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Dec 8, 2023

This seems not to be used anywhere other than benchmarks

Breaking as this is part of the public interface

@andiwand andiwand added this to the next milestone Dec 8, 2023
@github-actions github-actions bot added Component - Core Affects the Core module Component - Plugins Affects one or more Plugins labels Dec 8, 2023
Copy link

codecov bot commented Dec 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (66908fc) 48.84% compared to head (42d473b) 48.84%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2781   +/-   ##
=======================================
  Coverage   48.84%   48.84%           
=======================================
  Files         486      484    -2     
  Lines       28217    28160   -57     
  Branches    13321    13286   -35     
=======================================
- Hits        13782    13755   -27     
- Misses       4795     4798    +3     
+ Partials     9640     9607   -33     

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

@paulgessinger
Copy link
Member

I think this is used in the EDM4hep plugin at least.

@andiwand andiwand modified the milestones: next, v32.0.0 Dec 11, 2023
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.

ok let's do it

@kodiakhq kodiakhq bot merged commit 7c9479d into acts-project:main Dec 16, 2023
52 checks passed
@acts-project-service
Copy link
Collaborator

🔴 Athena integration test results [7c9479d]

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 Dec 16, 2023
@andiwand andiwand deleted the remove-covariance-transport branch December 16, 2023 12:41
kodiakhq bot pushed a commit that referenced this pull request Jan 9, 2024
kodiakhq bot pushed a commit that referenced this pull request Jan 19, 2024
In #2780 I realized that we do not call any of the functions in `JacobianEngine` on the current main branch and most of them are duplicated in `CovarianceEngine`. Here I try to unify the duplicated code again

blocked by
- #2782
- #2781
@paulgessinger paulgessinger added Breaks Athena build This PR breaks the Athena build and removed Breaks Athena build This PR breaks the Athena build labels Jan 23, 2024
LaraCalic pushed a commit to LaraCalic/acts that referenced this pull request Feb 10, 2024
This seems not to be used anywhere other than benchmarks

Breaking as this is part of the public interface
LaraCalic pushed a commit to LaraCalic/acts that referenced this pull request Feb 10, 2024
LaraCalic pushed a commit to LaraCalic/acts that referenced this pull request Feb 10, 2024
In acts-project#2780 I realized that we do not call any of the functions in `JacobianEngine` on the current main branch and most of them are duplicated in `CovarianceEngine`. Here I try to unify the duplicated code again

blocked by
- acts-project#2782
- acts-project#2781
kodiakhq bot pushed a commit that referenced this pull request Mar 28, 2024
…d of bound vector (#2811)

This avoid free->bound->free roundtrips and aligns the interface with `freeToBoundJacobian`

related issues
- #2810

blocked by
- #2789
- #2782
- #2781
kodiakhq bot pushed a commit that referenced this pull request Apr 9, 2024
I noticed that the concept of a curvilinear surface is a quite spread over the codebase and I wanted to improve this. Here I introduce a surface like class which does not actually inherit from `Acts::Surface` but shares some of the functionality. This way the jacobians are put in a single place and can be used from somewhere else in an expressive fashion.

Afterwards it might make sense to let the `CurvilinearTrackParameters` depend on this special surface instead of a `PlaneSurface` or to merge them with `BoundTrackParameters` completely.

related issues
- #2812 

blocked by
- #2789
- #2782
- #2781
- #2811
Ragansu pushed a commit to Ragansu/acts that referenced this pull request Apr 19, 2024
I noticed that the concept of a curvilinear surface is a quite spread over the codebase and I wanted to improve this. Here I introduce a surface like class which does not actually inherit from `Acts::Surface` but shares some of the functionality. This way the jacobians are put in a single place and can be used from somewhere else in an expressive fashion.

Afterwards it might make sense to let the `CurvilinearTrackParameters` depend on this special surface instead of a `PlaneSurface` or to merge them with `BoundTrackParameters` completely.

related issues
- acts-project#2812 

blocked by
- acts-project#2789
- acts-project#2782
- acts-project#2781
- acts-project#2811
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
…d of bound vector (acts-project#2811)

This avoid free->bound->free roundtrips and aligns the interface with `freeToBoundJacobian`

related issues
- acts-project#2810

blocked by
- acts-project#2789
- acts-project#2782
- acts-project#2781
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
I noticed that the concept of a curvilinear surface is a quite spread over the codebase and I wanted to improve this. Here I introduce a surface like class which does not actually inherit from `Acts::Surface` but shares some of the functionality. This way the jacobians are put in a single place and can be used from somewhere else in an expressive fashion.

Afterwards it might make sense to let the `CurvilinearTrackParameters` depend on this special surface instead of a `PlaneSurface` or to merge them with `BoundTrackParameters` completely.

related issues
- acts-project#2812 

blocked by
- acts-project#2789
- acts-project#2782
- acts-project#2781
- acts-project#2811
asalzburger pushed a commit to asalzburger/acts that referenced this pull request May 21, 2024
…d of bound vector (acts-project#2811)

This avoid free->bound->free roundtrips and aligns the interface with `freeToBoundJacobian`

related issues
- acts-project#2810

blocked by
- acts-project#2789
- acts-project#2782
- acts-project#2781
asalzburger pushed a commit to asalzburger/acts that referenced this pull request May 21, 2024
I noticed that the concept of a curvilinear surface is a quite spread over the codebase and I wanted to improve this. Here I introduce a surface like class which does not actually inherit from `Acts::Surface` but shares some of the functionality. This way the jacobians are put in a single place and can be used from somewhere else in an expressive fashion.

Afterwards it might make sense to let the `CurvilinearTrackParameters` depend on this special surface instead of a `PlaneSurface` or to merge them with `BoundTrackParameters` completely.

related issues
- acts-project#2812 

blocked by
- acts-project#2789
- acts-project#2782
- acts-project#2781
- acts-project#2811
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 - Plugins Affects one or more Plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants