-
Notifications
You must be signed in to change notification settings - Fork 157
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: Refitting algorithm #1940
feat: Refitting algorithm #1940
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't look through all of it yet. Looks good as far as I can tell, a few remarks here.
Also, can we hold this until after #1939, and move to the DataHandles immediately?
Examples/Algorithms/TrackFitting/include/ActsExamples/TrackFitting/TrackFitterFunction.hpp
Outdated
Show resolved
Hide resolved
Examples/Algorithms/TrackFitting/src/AlreadyCalibratedCalibrator.cpp
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## main #1940 +/- ##
=======================================
Coverage 49.83% 49.83%
=======================================
Files 421 421
Lines 23900 23900
Branches 10844 10844
=======================================
Hits 11910 11910
Misses 4367 4367
Partials 7623 7623
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this 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
📊 Physics performance monitoring for 4193decFull report VertexingSeedingCKFAmbiguity resolutionTruth tracking (Kalman Filter)Truth tracking (GSF) |
I'm currently working on implementing a few different measurement/cluster calibration methods for a bona fide calibration study. The easiest way to run such a study would be to have the option to swap the calibrators on the python side so one does not need to either recompile everytime or duplicate the TrackFitting example for each different methods. Towards that end, I propose the following changes: - Rename `MeasurementCalibrator` to `PassThroughCalibrator`; - Create an abstract base class `MeasurementCalibrator` with a calibrate method which takes a measurement container as input; - Created a `PairedMeasurementCalibrator` class that binds a calibrator to a `MeasurementContainer`, to pass to the actual fitter; - Add machinery to instantiate the calibrators outside of the `execute()` loop. #1940 introduced a alternate path using a RefittingCalibrator instead of a MeasurementCalibrator. I did _not_ touch this code since I'm not sure if it makes sense to unify it with what I have planned -- any thoughts @benjaminhuth ? I propose to leave this as-is for the time being and re-assess once I push actual calibrators (shortly, hopefully).
So far, real refitting of tracks was not possible in the examples framework. The direct-fitter option of the
ActsExamples::TrackFittingAlgorithm
was broken due to the fact, that only measurement surfaces were added to the surface sequence, and thus e.g. the traversed material could differ.This adds
ActsExamples::RefittingAlgorithm
, that uses the newActs::TrackContainer
infrastructure to enable a very simple interface: Just a input container and a output container.Furthermore, the
TrackFittingAlgorithm
looses thedirectFit
option. To make this work, also theTrackFitterFunction
had to be refactored and was pulled out of theTrackFittingAlgorithm
.