refactor: Refactor target direction logic in KF/CKF #2539
Conversation
github-actions
bot
added
Component - Core
|
Hello I am teaching this week, so I will look at this on Monday if it is fine |
Codecov Report
@@ Coverage Diff @@
## main #2539 +/- ##
==========================================
- Coverage 49.79% 49.74% -0.05%
==========================================
Files 466 466
Lines 26326 26354 +28
Branches 12095 12103 +8
==========================================
+ Hits 13108 13111 +3
- Misses 4615 4638 +23
- Partials 8603 8605 +2
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
I will need to cross check but does the target surface has to be a perigee surface at the beamline can't it be any arbitrary surface ? Because I can see a few use case where you would want to have it behind the last measurements. |
I agree, it might be generally useful but the current implementation is unstable for our usual setup with the perigee in front of the first sensitive. If the target surface is behind the last sensitive smoothing should not be necessary but is currently coupled to the target propagation. The problem I encounter is that the perigee surface might be "closer" to the last track state VS the first track state because we intersect with the PCA and not a plane surface in that case. Apart from that it is not clear what happens if we cannot reach the target source or if we are not in the bounds. These details should be handled by the user IMO because it would be way to complicated to handle all of them in a core algorithm. |
|
@paulgessinger @asalzburger @XiaocongAi is someone aware if users rely on this feature? @Corentin-Allaire is (rightfully) concerned that this will break user code |
To be more precise i there any users for which the target surface is not before the first layer (the main thing that would come to mind for me is test beam setup or maybe fix target) |
|
I believe @pbutti needs the last measurement surface, but I think he's picking it up manually anyway. Can you confirm? |
|
I may be misunderstanding the use case, but it sounds like this also might affect cosmic track fitting where a cosmic ray traverses your entire detector and you want to the track parameters determined at some target surface outside the detector at an arbitrary location. |
andiwand
changed the title
andiwand
changed the title
acts-project-service
added
the
Fails Athena tests
After #2539 I want to finally refactor the CKF and remove the smoothing and the reference surface targeting from the core algorithm in order to make it more composable and less complicated. Before the CKF was basically only usable with smoothing turned on otherwise we would not receive any tracks. Now it always runs without smoothing and the user can decide what and how to smooth. Afterwards the user can decide which track state they want to extrapolate to a reference surface if necessary. IMO this makes the algorithm flow much easier to comprehend and gives the user more flexibility. This also makes an implementation of a two way finding easier as discovered in #2717 blocked by - #2723 --- Users can smooth and extrapolate tracks after the CKF returns them. Extrapolation can be achieved using the `Propagator` while smoothing can be done by the `GainMatrixSmoother`. For convenience this PR also includes helper functions to smooth and extrapolate single tracks or a whole track container. Example of smoothing and extrapolating tracks after the CKF https://github.com/acts-project/acts/blob/98186891d4bdc5b211f6af3e96ca59ace7c315cc/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp#L172-L189
…ject#2722) After acts-project#2539 I want to finally refactor the CKF and remove the smoothing and the reference surface targeting from the core algorithm in order to make it more composable and less complicated. Before the CKF was basically only usable with smoothing turned on otherwise we would not receive any tracks. Now it always runs without smoothing and the user can decide what and how to smooth. Afterwards the user can decide which track state they want to extrapolate to a reference surface if necessary. IMO this makes the algorithm flow much easier to comprehend and gives the user more flexibility. This also makes an implementation of a two way finding easier as discovered in acts-project#2717 blocked by - acts-project#2723 --- Users can smooth and extrapolate tracks after the CKF returns them. Extrapolation can be achieved using the `Propagator` while smoothing can be done by the `GainMatrixSmoother`. For convenience this PR also includes helper functions to smooth and extrapolate single tracks or a whole track container. Example of smoothing and extrapolating tracks after the CKF https://github.com/acts-project/acts/blob/98186891d4bdc5b211f6af3e96ca59ace7c315cc/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp#L172-L189
…ject#2722) After acts-project#2539 I want to finally refactor the CKF and remove the smoothing and the reference surface targeting from the core algorithm in order to make it more composable and less complicated. Before the CKF was basically only usable with smoothing turned on otherwise we would not receive any tracks. Now it always runs without smoothing and the user can decide what and how to smooth. Afterwards the user can decide which track state they want to extrapolate to a reference surface if necessary. IMO this makes the algorithm flow much easier to comprehend and gives the user more flexibility. This also makes an implementation of a two way finding easier as discovered in acts-project#2717 blocked by - acts-project#2723 --- Users can smooth and extrapolate tracks after the CKF returns them. Extrapolation can be achieved using the `Propagator` while smoothing can be done by the `GainMatrixSmoother`. For convenience this PR also includes helper functions to smooth and extrapolate single tracks or a whole track container. Example of smoothing and extrapolating tracks after the CKF https://github.com/acts-project/acts/blob/98186891d4bdc5b211f6af3e96ca59ace7c315cc/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp#L172-L189
Not sure what the use case for this was but to me it seems like the target propagation logic is flawed in KF/CKF. If we propagate to target from the last state the smoothing seems unnecessary. Also since the target surface is a perigee surface for us there are edge cases where the last state has a smaller "distance" to the perigee than the first one.
In general I'd say the user should take such a decision but this is too big of a change at the moment.
discovered with #2518