fix: Use material towards target in CKF (#2322)

reopened #2318 after #2321

The CKF did not pick up the material in-between the first surface and the target surface. This PR is trying to fix that.

Note: I had to use a not so nice hack and call the navigation pre step routine before we step towards the target. I don't see an easy way around that.

blocked by
- #2336

Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp

@@ -354,12 +354,12 @@ class CombinatorialKalmanFilter {
     /// @param stepper is the stepper in use
     /// @param navigator is the navigator in use
     /// @param result is the mutable result state object
-    /// @param logger the logger object to be used
+    /// @param _logger the logger object associated to the @c Propagator. CKF uses its own logger instance.
     template <typename propagator_state_t, typename stepper_t,
               typename navigator_t>
     void operator()(propagator_state_t& state, const stepper_t& stepper,
                     const navigator_t& navigator, result_type& result,
-                    const Logger& logger) const {
+                    const Logger& _logger) const {
       assert(result.fittedStates && "No MultiTrajectory set");
 
       if (result.finished) {
@@ -513,6 +513,16 @@ class CombinatorialKalmanFilter {
                 result.result = res.error();
               }
               result.smoothed = true;
+
+              // TODO another ugly control flow hack
+              navigator.preStep(state, stepper);
+            }
+
+            if (result.smoothed) {
+              // Update state and stepper with material effects
+              materialInteractor(navigator.currentSurface(state.navigation),
+                                 state, stepper, navigator,
+                                 MaterialUpdateStage::FullUpdate);
             }
 
             // -> then progress to target/reference surface and built the final
@@ -555,7 +565,7 @@ class CombinatorialKalmanFilter {
 
                 // TODO this is kinda silly but I dont see a better solution
                 // with the current CKF control flow
-                operator()(state, stepper, navigator, result, logger);
+                operator()(state, stepper, navigator, result, _logger);
               } else {
                 ACTS_VERBOSE("Finish Kalman filtering and smoothing");
                 // Remember that track finding is done
@@ -584,7 +594,7 @@ class CombinatorialKalmanFilter {
       auto currentState =
           result.fittedStates->getTrackState(result.activeTips.back().first);
 
-      // Update the stepping state
+      // Reset the stepping state
       stepper.resetState(state.stepping, currentState.filtered(),
                          currentState.filteredCovariance(),
                          currentState.referenceSurface(),
@@ -787,11 +797,10 @@ class CombinatorialKalmanFilter {
             nBranchesOnSurface = 0;
           }
         }
-        if (surface->surfaceMaterial() != nullptr) {
-          // Update state and stepper with material effects
-          materialInteractor(surface, state, stepper, navigator,
-                             MaterialUpdateStage::FullUpdate);
-        }
+
+        // Update state and stepper with material effects
+        materialInteractor(surface, state, stepper, navigator,
+                           MaterialUpdateStage::FullUpdate);
       } else {
         // Neither measurement nor material on surface, this branch is still
         // valid. Count the branch on current surface
@@ -1083,10 +1092,14 @@ class CombinatorialKalmanFilter {
                             const stepper_t& stepper,
                             const navigator_t& navigator,
                             const MaterialUpdateStage& updateStage) const {
+      if (surface == nullptr) {
+        return;
+      }
+
       // Indicator if having material
       bool hasMaterial = false;
 
-      if (surface and surface->surfaceMaterial()) {
+      if (surface->surfaceMaterial() != nullptr) {
         // Prepare relevant input particle properties
         detail::PointwiseMaterialInteraction interaction(surface, state,
                                                          stepper);
@@ -1104,7 +1117,7 @@ class CombinatorialKalmanFilter {
                        << surface->geometryId()
                        << " at update stage: " << updateStage << " are :");
           ACTS_VERBOSE("eLoss = "
-                       << interaction.Eloss << ", "
+                       << interaction.Eloss * interaction.navDir << ", "
                        << "variancePhi = " << interaction.variancePhi << ", "
                        << "varianceTheta = " << interaction.varianceTheta
                        << ", "
@@ -1217,49 +1230,41 @@ class CombinatorialKalmanFilter {
       // assumes the target surface is not within the first and the last
       // smoothed measurement state. Also, whether the intersection is on
       // surface is not checked here.
-      bool reverseDirection = false;
       bool closerToFirstCreatedState =
-          (std::abs(firstIntersection.pathLength()) <=
-           std::abs(lastIntersection.pathLength()));
+          std::abs(firstIntersection.pathLength()) <=
+          std::abs(lastIntersection.pathLength());
+      bool reverseDirection = false;
       if (closerToFirstCreatedState) {
-        stepper.update(state.stepping, firstParams,
-                       firstCreatedState.smoothed(),
-                       firstCreatedState.smoothedCovariance(),
-                       firstCreatedState.referenceSurface());
+        stepper.resetState(state.stepping, firstCreatedState.smoothed(),
+                           firstCreatedState.smoothedCovariance(),
+                           firstCreatedState.referenceSurface(),
+                           state.options.maxStepSize);
         reverseDirection = firstIntersection.pathLength() < 0;
       } else {
-        stepper.update(state.stepping, lastParams,
-                       lastCreatedMeasurement.smoothed(),
-                       lastCreatedMeasurement.smoothedCovariance(),
-                       lastCreatedMeasurement.referenceSurface());
+        stepper.resetState(state.stepping, lastCreatedMeasurement.smoothed(),
+                           lastCreatedMeasurement.smoothedCovariance(),
+                           lastCreatedMeasurement.referenceSurface(),
+                           state.options.maxStepSize);
         reverseDirection = lastIntersection.pathLength() < 0;
       }
-      const auto& surface = closerToFirstCreatedState
-                                ? firstCreatedState.referenceSurface()
-                                : lastCreatedMeasurement.referenceSurface();
-      ACTS_VERBOSE(
-          "Smoothing successful, updating stepping state to smoothed "
-          "parameters at surface "
-          << surface.geometryId() << ". Prepared to reach the target surface.");
-
       // Reverse the navigation direction if necessary
       if (reverseDirection) {
         ACTS_VERBOSE(
             "Reverse navigation direction after smoothing for reaching the "
             "target surface");
         state.options.direction = state.options.direction.invert();
       }
-      // Reinitialize the stepping jacobian
-      state.stepping.jacobian = BoundMatrix::Identity();
-      state.stepping.jacTransport = FreeMatrix::Identity();
-      state.stepping.derivative = FreeVector::Zero();
-      // Reset the step size
-      state.stepping.stepSize = ConstrainedStep(state.options.maxStepSize);
-      // Set accumulatd path to zero before targeting surface
-      state.stepping.pathAccumulated = 0.;
+      const auto& surface = closerToFirstCreatedState
+                                ? firstCreatedState.referenceSurface()
+                                : lastCreatedMeasurement.referenceSurface();
+      ACTS_VERBOSE(
+          "Smoothing successful, updating stepping state to smoothed "
+          "parameters at surface "
+          << surface.geometryId() << ". Prepared to reach the target surface.");
 
       // Reset the navigation state to enable propagation towards the target
       // surface
+      // Set targetSurface to nullptr as it is handled manually in the actor
       navigator.resetState(
           state.navigation, state.geoContext, stepper.position(state.stepping),
           state.options.direction * stepper.direction(state.stepping), &surface,

Core/include/Acts/TrackFitting/KalmanFitter.hpp

@@ -441,6 +441,14 @@ class KalmanFitter {
       // - Progress to target/reference surface and built the final track
       // parameters
       if (result.smoothed or result.reversed) {
+        if (result.smoothed) {
+          // Update state and stepper with material effects
+          // Only for smoothed as reverse filtering will handle this separately
+          materialInteractor(navigator.currentSurface(state.navigation), state,
+                             stepper, navigator,
+                             MaterialUpdateStage::FullUpdate);
+        }
+
         if (targetSurface == nullptr) {
           // If no target surface provided:
           // -> Return an error when using reversed filtering mode
@@ -963,11 +971,11 @@ class KalmanFitter {
       // assumes the target surface is not within the first and the last
       // smoothed measurement state. Also, whether the intersection is on
       // surface is not checked here.
-      bool reverseDirection = false;
-      bool closerTofirstCreatedState =
+      bool closerToFirstCreatedState =
           std::abs(firstIntersection.pathLength()) <=
           std::abs(lastIntersection.pathLength());
-      if (closerTofirstCreatedState) {
+      bool reverseDirection = false;
+      if (closerToFirstCreatedState) {
         stepper.resetState(state.stepping, firstCreatedState.smoothed(),
                            firstCreatedState.smoothedCovariance(),
                            firstCreatedState.referenceSurface(),
@@ -980,24 +988,24 @@ class KalmanFitter {
                            state.options.maxStepSize);
         reverseDirection = lastIntersection.pathLength() < 0;
       }
-      const auto& surface = closerTofirstCreatedState
-                                ? firstCreatedState.referenceSurface()
-                                : lastCreatedMeasurement.referenceSurface();
-      ACTS_VERBOSE(
-          "Smoothing successful, updating stepping state to smoothed "
-          "parameters at surface "
-          << surface.geometryId() << ". Prepared to reach the target surface.");
-
       // Reverse the navigation direction if necessary
       if (reverseDirection) {
         ACTS_VERBOSE(
             "Reverse navigation direction after smoothing for reaching the "
             "target surface");
         state.options.direction = state.options.direction.invert();
       }
+      const auto& surface = closerToFirstCreatedState
+                                ? firstCreatedState.referenceSurface()
+                                : lastCreatedMeasurement.referenceSurface();
+      ACTS_VERBOSE(
+          "Smoothing successful, updating stepping state to smoothed "
+          "parameters at surface "
+          << surface.geometryId() << ". Prepared to reach the target surface.");
 
       // Reset the navigation state to enable propagation towards the target
       // surface
+      // Set targetSurface to nullptr as it is handled manually in the actor
       navigator.resetState(
           state.navigation, state.geoContext, stepper.position(state.stepping),
           state.options.direction * stepper.direction(state.stepping), &surface,

Examples/Python/tests/root_file_hashes.txt

@@ -23,13 +23,13 @@ test_itk_seeding__particles_initial.root: 88315e93ed4cb5d40a8721502048a9d1fc100e
 test_propagation__propagation_steps.root: 174301b25784dbb881196b658f2d7f99c0a2ea688a0129e6110fc19aa5cf8e54
 test_material_recording__geant4_material_tracks.root: e411152d370775463c22b19a351dfc7bfe40b51985e10a7c1a010aebde80715d
 test_truth_tracking_kalman[generic-0.0]__trackstates_fitter.root: 0cdfeb13eaff45df38451820fcc15a7dfb40e55dab220e6e1d4d6f027ee8d4c8
-test_truth_tracking_kalman[generic-0.0]__tracksummary_fitter.root: 135ce6fbbe1789c56d42a834e148e6ce395c7425373989f8c03e6c32c989fe94
+test_truth_tracking_kalman[generic-0.0]__tracksummary_fitter.root: b0d5d7ee2e742aef97db99ed34dce0024cfa947a60f57b16c2e4033c48656cf7
 test_truth_tracking_kalman[generic-0.0]__performance_track_finder.root: ada9cda2ec3c648b144bdaa081d6eff923c80f3d484c4196006e847428cf67a8
 test_truth_tracking_kalman[generic-1000.0]__trackstates_fitter.root: 5d54954ad21b8153b1788ccc296680a50969c761924f1c5db0600033d4577a3c
 test_truth_tracking_kalman[generic-1000.0]__tracksummary_fitter.root: 3ecd6851e1633103b9ae20140aa8f6a5a27aecc777177cd83078ed292544d9ae
 test_truth_tracking_kalman[generic-1000.0]__performance_track_finder.root: ada9cda2ec3c648b144bdaa081d6eff923c80f3d484c4196006e847428cf67a8
 test_truth_tracking_kalman[odd-0.0]__trackstates_fitter.root: 7afa54c9f2c989e11236861b672e822f881443c5976dfcb65cebbab155280a83
-test_truth_tracking_kalman[odd-0.0]__tracksummary_fitter.root: 85cdf6aced8c7f7eb1c156d912f6be6a7ab1370b4c72955c0bd7b09095df9a60
+test_truth_tracking_kalman[odd-0.0]__tracksummary_fitter.root: 7432fe3e168733085d4b0a83229777c08f9f99ee3f6ebf9ade8072d9b496c46b
 test_truth_tracking_kalman[odd-0.0]__performance_track_finder.root: 39aec6316cceb90e314e16b02947faa691c18f57c3a851a25e547a8fc05a4593
 test_truth_tracking_kalman[odd-1000.0]__trackstates_fitter.root: 04ec5361f32eae9151497c54d89c2a0b03a088050be7a37f521f65cd3abf85c8
 test_truth_tracking_kalman[odd-1000.0]__tracksummary_fitter.root: 2a6de6dedd20acae66eef29649a0966f0b83ea00cee9475fbaaca696058b2525
@@ -50,21 +50,21 @@ test_digitization_example_input[smeared]__measurements.root: df0e31749f64f52b791
 test_digitization_example_input[geometric]__particles.root: 8549ba6e20338004ab8ba299fc65e1ee5071985b46df8f77f887cb6fef56a8ec
 test_digitization_example_input[geometric]__measurements.root: b4dba5bb5e60d4540c1d07fb297b2add19d779dfcd29737c8d44f208ff0c0522
 test_ckf_tracks_example[generic-full_seeding]__trackstates_ckf.root: 1786b1f1e90a8fe2ad6f534072506bacf8dbcf1a7e25c283a5ab261a319479fb
-test_ckf_tracks_example[generic-full_seeding]__tracksummary_ckf.root: 9e950e1bc6cecd429b4e66ff6bc3ceb6371f1f1d8ad22f080438db4e2d10c19d
+test_ckf_tracks_example[generic-full_seeding]__tracksummary_ckf.root: 359be063e44247439490d4ae99fa3c9c240e8c0777b27421b26b63f62c85e2f0
 test_ckf_tracks_example[generic-full_seeding]__performance_seeding_trees.root: 0e0676ffafdb27112fbda50d1cf627859fa745760f98073261dcf6db3f2f991e
 test_ckf_tracks_example[generic-truth_estimated]__trackstates_ckf.root: 7dd654c2223f3c53eac4ba496b3dec3739d29a443e9a77e718981bad44b95f1c
-test_ckf_tracks_example[generic-truth_estimated]__tracksummary_ckf.root: e9886b61af4934c1b2a825719ea69ec3b69b245927cb22d8cca5071612609fe5
+test_ckf_tracks_example[generic-truth_estimated]__tracksummary_ckf.root: 49b39633c0794f7c215b07626dcaba026e4358a795198b29fb0b59fde41abff4
 test_ckf_tracks_example[generic-truth_estimated]__performance_seeding.root: 1facb05c066221f6361b61f015cdf0918e94d9f3fce2269ec7b6a4dffeb2bc7e
 test_ckf_tracks_example[generic-truth_smeared]__trackstates_ckf.root: 0351e6e87bbda82c666e3580f8437fa9736f9d95ec4dd8658129bc73bb3fbf6c
-test_ckf_tracks_example[generic-truth_smeared]__tracksummary_ckf.root: 21eb776b8b6974543c0bfed6cdc18773a9f20629bed5d99154f2ded79a4297cd
+test_ckf_tracks_example[generic-truth_smeared]__tracksummary_ckf.root: d0d843bdb92b39dea1151894f411d645ccaae2423f9293e3251d082835b126bb
 test_ckf_tracks_example[odd-full_seeding]__trackstates_ckf.root: bf564fc788eed6e72791d4eac639bd439ba2e167554cee4edb6a6162cec22089
-test_ckf_tracks_example[odd-full_seeding]__tracksummary_ckf.root: f107b73de07e5fb035c2a3e50b2d2928df77d7ce6579341bc5a227485972c878
+test_ckf_tracks_example[odd-full_seeding]__tracksummary_ckf.root: cd39a64fecd59efd9e007eba9e3457be9bbcf534ebefa02617b9228d107f5e41
 test_ckf_tracks_example[odd-full_seeding]__performance_seeding_trees.root: 43c58577aafe07645e5660c4f43904efadf91d8cda45c5c04c248bbe0f59814f
 test_ckf_tracks_example[odd-truth_estimated]__trackstates_ckf.root: 3defdfb49c4942e53e5cda8f2cd01475ce2bfe1b92b6745d83ea17d9c2ad535f
-test_ckf_tracks_example[odd-truth_estimated]__tracksummary_ckf.root: b9a5e794e3d8c3c64426efb4abe2ce38e338de96f3de4f751b238b5ae1997189
+test_ckf_tracks_example[odd-truth_estimated]__tracksummary_ckf.root: 0be2a992ca77ab7fbf27dc2fc04ddf65add1ac4934e51f71a1ecc9edea871ef6
 test_ckf_tracks_example[odd-truth_estimated]__performance_seeding.root: 1a36b7017e59f1c08602ef3c2cb0483c51df248f112e3780c66594110719c575
 test_ckf_tracks_example[odd-truth_smeared]__trackstates_ckf.root: d4b47a6c9621847be84a17a21fb929a050c49d0c7b3d944bf14fee365478609d
-test_ckf_tracks_example[odd-truth_smeared]__tracksummary_ckf.root: e01769e412c89fe73dac106465f2d6b13744da2d8e1b6d73edb8f91830024cd6
+test_ckf_tracks_example[odd-truth_smeared]__tracksummary_ckf.root: 84b381367e8722a9dfcffa3f5d72e8b0c9a22f9df460f502d31a13e52b5ac36c
 test_vertex_fitting_reading[Truth-False-100]__performance_vertexing.root: 76ef6084d758dfdfc0151ddec2170e12d73394424e3dac4ffe46f0f339ec8293
 test_vertex_fitting_reading[Iterative-False-100]__performance_vertexing.root: 60372210c830a04f95ceb78c6c68a9b0de217746ff59e8e73053750c837b57eb
 test_vertex_fitting_reading[Iterative-True-100]__performance_vertexing.root: e34f217d524a5051dbb04a811d3407df3ebe2cc4bb7f54f6bda0847dbd7b52c3