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

feat: GX2F (Global Chi2 Fitter) | Gχ²F #1580

Merged
merged 65 commits into from
Nov 3, 2022

Conversation

AJPfleger
Copy link
Contributor

@AJPfleger AJPfleger commented Oct 6, 2022

A clean PR for the GX2F. This is the completion of PR #1099 since, some segfaults on mac were causing problems (read there for more info on that issue).

The GX2F consists now of:

  • core algorithm Chi2Fitter.hpp
  • simple UnitTest

Update 2022-10-26: We found a second segfault and fixed it with this commit
Update 2022-10-28: move to namespace Experimental
Update 2022-10-31: adapt for PR #1512 (thank you @paulgessinger)
Update 2022-10-31: adapt for PR #1610

@AJPfleger AJPfleger added this to the next milestone Oct 6, 2022
@AJPfleger AJPfleger added Component - Core Affects the Core module Component - Examples Affects the Examples module labels Oct 6, 2022
@codecov
Copy link

codecov bot commented Oct 6, 2022

Codecov Report

Merging #1580 (2adbe34) into main (ec15a62) will decrease coverage by 0.04%.
The diff coverage is 31.73%.

@@            Coverage Diff             @@
##             main    #1580      +/-   ##
==========================================
- Coverage   48.58%   48.54%   -0.05%     
==========================================
  Files         381      384       +3     
  Lines       20833    21041     +208     
  Branches     9557     9693     +136     
==========================================
+ Hits        10122    10214      +92     
+ Misses       4108     4105       -3     
- Partials     6603     6722     +119     
Impacted Files Coverage Δ
Core/src/TrackFitting/Chi2FitterError.cpp 0.00% <0.00%> (ø)
Core/include/Acts/TrackFitting/Chi2Fitter.hpp 32.98% <32.98%> (ø)
...de/Acts/TrackFitting/detail/VoidChi2Components.hpp 50.00% <50.00%> (ø)
Core/include/Acts/Propagator/Navigator.hpp 55.65% <0.00%> (+0.88%) ⬆️
Core/include/Acts/Propagator/ConstrainedStep.hpp 72.34% <0.00%> (+2.12%) ⬆️
Core/src/Utilities/Logger.cpp 39.13% <0.00%> (+8.69%) ⬆️
Core/src/Definitions/Common.cpp 32.00% <0.00%> (+32.00%) ⬆️
Core/include/Acts/Surfaces/Surface.hpp 66.66% <0.00%> (+33.33%) ⬆️
Core/src/Surfaces/RectangleBounds.cpp 86.66% <0.00%> (+53.33%) ⬆️

📣 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.

just glanced over the code. feel free to just archive my comments if they don't make sense

Tests/UnitTests/Core/TrackFitting/Chi2FitterTests.cpp Outdated Show resolved Hide resolved
Examples/Scripts/Python/truth_tracking_chi2.py Outdated Show resolved Hide resolved
Core/include/Acts/TrackFitting/Chi2Fitter.hpp Show resolved Hide resolved
Core/include/Acts/TrackFitting/Chi2Fitter.hpp Show resolved Hide resolved
Core/include/Acts/TrackFitting/Chi2Fitter.hpp Outdated Show resolved Hide resolved
Core/include/Acts/TrackFitting/Chi2Fitter.hpp Outdated Show resolved Hide resolved
Core/include/Acts/TrackFitting/Chi2Fitter.hpp Show resolved Hide resolved
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.

I am ok to get this in if we put it in a follow-up PR into Experimental and then start tackling the next parts.

Core/include/Acts/TrackFitting/Chi2Fitter.hpp Show resolved Hide resolved
Core/include/Acts/TrackFitting/Chi2Fitter.hpp Show resolved Hide resolved
Core/include/Acts/TrackFitting/Chi2Fitter.hpp Outdated Show resolved Hide resolved
Core/include/Acts/TrackFitting/Chi2Fitter.hpp Show resolved Hide resolved
Core/include/Acts/TrackFitting/Chi2Fitter.hpp Show resolved Hide resolved
@paulgessinger
Copy link
Member

paulgessinger commented Nov 1, 2022

I'll merge this manually when the CI completes, so we can properly add a reference to @r4lv in the commit.

@paulgessinger paulgessinger merged commit 890eff9 into acts-project:main Nov 3, 2022
@paulgessinger paulgessinger modified the milestones: next, v21.0.0 Nov 3, 2022
kodiakhq bot pushed a commit that referenced this pull request Jul 13, 2023
…2302)

Clean removal of my old gx2f implementation from PR #1580 to make space for my new implementation. The new one will work, I promise.
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Jul 24, 2023
…cts-project#2302)

Clean removal of my old gx2f implementation from PR acts-project#1580 to make space for my new implementation. The new one will work, I promise.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants