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!: Put GSF in namespace Acts::Experimental #1609

Merged

Conversation

benjaminhuth
Copy link
Member

Since the GSF is not yet fully functional, it is put in the namespace Acts::Experimental.

Also adds some hints in the documentation, that components in this namespace are not yet production read, and also not part of the public API (I think that is reasonable, since clients should not rely on experimental code).

However, since this would be a new rule, this change is breaking.

@benjaminhuth benjaminhuth added Component - Core Affects the Core module Component - Documentation Affects the documentation labels Oct 20, 2022
@benjaminhuth benjaminhuth added this to the next milestone Oct 20, 2022
@benjaminhuth benjaminhuth changed the title !refactor: Put GSF in namespace Acts::Experimental refactor!: Put GSF in namespace Acts::Experimental Oct 20, 2022
@codecov
Copy link

codecov bot commented Oct 20, 2022

Codecov Report

Merging #1609 (f603349) into main (1d7e9b2) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

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

@@            Coverage Diff             @@
##             main    #1609      +/-   ##
==========================================
- Coverage   48.66%   48.63%   -0.03%     
==========================================
  Files         381      381              
  Lines       20776    20786      +10     
  Branches     9517     9519       +2     
==========================================
  Hits        10110    10110              
- Misses       4095     4105      +10     
  Partials     6571     6571              
Impacted Files Coverage Δ
...re/include/Acts/TrackFitting/GaussianSumFitter.hpp 64.63% <ø> (ø)
Core/include/Acts/TrackFitting/GsfOptions.hpp 100.00% <ø> (ø)
Core/include/Acts/TrackFitting/detail/GsfActor.hpp 43.30% <0.00%> (-0.18%) ⬇️
.../include/Acts/TrackFitting/detail/GsfSmoothing.hpp 35.23% <0.00%> (ø)
Core/src/TrackFitting/GsfError.cpp 0.00% <0.00%> (ø)
Core/include/Acts/Seeding/SeedFilter.hpp 0.00% <0.00%> (ø)
Core/include/Acts/Seeding/SeedFilter.ipp 0.00% <0.00%> (ø)
Core/include/Acts/Seeding/SeedFinder.ipp 0.00% <0.00%> (ø)

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

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. not sure if anybody else wants to take a look as this is a breaking change. cc @asalzburger @paulgessinger

@benjaminhuth
Copy link
Member Author

Maybe we should also quickly discuss this in tomorrows developer meeting, since it proposes changes to the definition of the public API / the versioning...

@github-actions
Copy link

📊 Physics performance monitoring for d18177a

Full report
CKF: seeded, truth smeared, truth estimated
IVF: seeded, truth smeared, truth estimated
Ambiguity resolution
Truth tracking

Vertexing

IVF seeded

IVF truth smeared

IVF truth estimated

CKF

seeded

truth smeared

truth estimated

Ambiguity resolution

seeded

Truth tracking

Truth tracking

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.

Reapproving, after AStefl had looked over it

@asalzburger asalzburger merged commit f6f0e1f into acts-project:main Oct 26, 2022
@paulgessinger paulgessinger modified the milestones: next, v21.0.0 Nov 3, 2022
kodiakhq bot pushed a commit that referenced this pull request Nov 7, 2022
This PR refactors a bit the Bethe Heitler Approximation
* moving it out of namespace `detail`, since its in fact part of the API of the GSF
* allow loading parameterizations from the python examples

Already contains #1609
@benjaminhuth benjaminhuth deleted the refactor/make-gsf-experimental branch November 9, 2022 13:28
kodiakhq bot pushed a commit that referenced this pull request Nov 11, 2022
This is a major refactoring of the GSF. It changes the propagation structure, and the way the GSF records states to the `MultiTrajectory`. It also removes the smoothing code, since it is not really useful at the moment, as the component states are not exported anyways.

should be merged after #1609 and #1627 and #1628 

We should add performance monitoring in this or a later PR

Fixes #1666
CarloVarni pushed a commit to CarloVarni/acts that referenced this pull request Dec 22, 2022
This is a major refactoring of the GSF. It changes the propagation structure, and the way the GSF records states to the `MultiTrajectory`. It also removes the smoothing code, since it is not really useful at the moment, as the component states are not exported anyways.

should be merged after acts-project#1609 and acts-project#1627 and acts-project#1628 

We should add performance monitoring in this or a later PR

Fixes acts-project#1666
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 - Documentation Affects the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants