refactor: comment and unify the creation and filling of trackStatePro…

…xy (KF) (#2499) Makes the generation and filling of the trackStateProxy, look the same at all places, where it occurs. I tried to make it more readable by updating the comments and adding more scopes for temporary variables that are used for the filling. This should make it more easy to understand what's going on for the next user, because now it is clearer that we are: 1. generating a trackstate 2. fetching values from the propagation 3. filling them in 4. adding flags
acts-project · Oct 25, 2023 · e0d26d8 · e0d26d8
1 parent b4be75f
commit e0d26d8
Show file tree

Hide file tree

Showing 3 changed files with 130 additions and 105 deletions.
diff --git a/Core/include/Acts/EventData/TrackStatePropMask.hpp b/Core/include/Acts/EventData/TrackStatePropMask.hpp
@@ -31,7 +31,6 @@ enum struct TrackStatePropMask : std::uint8_t {
   Filtered = 1 << 1,
   Smoothed = 1 << 2,
   Jacobian = 1 << 3,
-
   Calibrated = 1 << 4,
 
   All = std::numeric_limits<std::uint8_t>::max(),  // should be all ones

diff --git a/Core/include/Acts/TrackFitting/KalmanFitter.hpp b/Core/include/Acts/TrackFitting/KalmanFitter.hpp
@@ -734,31 +734,40 @@ class KalmanFitter {
         materialInteractor(surface, state, stepper, navigator,
                            MaterialUpdateStage::PreUpdate);
 
-        // Bind the transported state to the current surface
-        auto res = stepper.boundState(state.stepping, *surface, false);
-        if (!res.ok()) {
-          return res.error();
-        }
-
-        auto& [boundParams, jacobian, pathLength] = *res;
-
-        // Create a detached track state proxy
-        auto tempTrackTip =
-            result.fittedStates->addTrackState(TrackStatePropMask::All);
-
-        // Get the detached track state proxy back
-        auto trackStateProxy = result.fittedStates->getTrackState(tempTrackTip);
+        auto fittedStates = *result.fittedStates;
+
+        // Add a <mask> TrackState entry multi trajectory. This allocates
+        // storage for all components, which we will set later.
+        TrackStatePropMask mask = TrackStatePropMask::All;
+        const size_t currentTrackIndex = fittedStates.addTrackState(
+            mask, Acts::MultiTrajectoryTraits::kInvalid);
+
+        // now get track state proxy back
+        typename traj_t::TrackStateProxy trackStateProxy =
+            fittedStates.getTrackState(currentTrackIndex);
+
+        // Set the trackStateProxy components with the state from the ongoing
+        // propagation
+        {
+          trackStateProxy.setReferenceSurface(surface->getSharedPtr());
+          // Bind the transported state to the current surface
+          auto res = stepper.boundState(state.stepping, *surface, false,
+                                        freeToBoundCorrection);
+          if (!res.ok()) {
+            return res.error();
+          }
+          auto& [boundParams, jacobian, pathLength] = *res;
 
-        trackStateProxy.setReferenceSurface(surface->getSharedPtr());
+          // Fill the track state
+          trackStateProxy.predicted() = std::move(boundParams.parameters());
+          if (boundParams.covariance().has_value()) {
+            trackStateProxy.predictedCovariance() =
+                std::move(*boundParams.covariance());
+          }
 
-        // Fill the track state
-        trackStateProxy.predicted() = std::move(boundParams.parameters());
-        if (boundParams.covariance().has_value()) {
-          trackStateProxy.predictedCovariance() =
-              std::move(*boundParams.covariance());
+          trackStateProxy.jacobian() = std::move(jacobian);
+          trackStateProxy.pathLength() = std::move(pathLength);
         }
-        trackStateProxy.jacobian() = std::move(jacobian);
-        trackStateProxy.pathLength() = std::move(pathLength);
 
         // We have predicted parameters, so calibrate the uncalibrated input
         // measurement

diff --git a/Core/include/Acts/TrackFitting/detail/KalmanUpdateHelpers.hpp b/Core/include/Acts/TrackFitting/detail/KalmanUpdateHelpers.hpp
@@ -1,6 +1,6 @@
 // This file is part of the Acts project.
 //
-// Copyright (C) 2021 CERN for the benefit of the Acts project
+// Copyright (C) 2021-2023 CERN for the benefit of the Acts project
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -42,74 +42,89 @@ auto kalmanHandleMeasurement(
     const size_t lastTrackIndex, bool doCovTransport, const Logger &logger,
     const FreeToBoundCorrection &freeToBoundCorrection = FreeToBoundCorrection(
         false)) -> Result<typename traj_t::TrackStateProxy> {
-  // Bind the transported state to the current surface
-  auto res = stepper.boundState(state.stepping, surface, doCovTransport,
-                                freeToBoundCorrection);
-  if (!res.ok()) {
-    return res.error();
-  }
-  auto &[boundParams, jacobian, pathLength] = *res;
-
-  // add a full TrackState entry multi trajectory
-  // (this allocates storage for all components, we will set them later)
-  const auto newTrackIndex =
-      fittedStates.addTrackState(TrackStatePropMask::All, lastTrackIndex);
+  // Add a <mask> TrackState entry multi trajectory. This allocates storage for
+  // all components, which we will set later.
+  TrackStatePropMask mask = TrackStatePropMask::All;
+  const size_t currentTrackIndex =
+      fittedStates.addTrackState(mask, lastTrackIndex);
 
   // now get track state proxy back
-  auto trackStateProxy = fittedStates.getTrackState(newTrackIndex);
+  typename traj_t::TrackStateProxy trackStateProxy =
+      fittedStates.getTrackState(currentTrackIndex);
+
+  // Set the trackStateProxy components with the state from the ongoing
+  // propagation
+  {
+    trackStateProxy.setReferenceSurface(surface.getSharedPtr());
+    // Bind the transported state to the current surface
+    auto res = stepper.boundState(state.stepping, surface, doCovTransport,
+                                  freeToBoundCorrection);
+    if (!res.ok()) {
+      ACTS_ERROR("Propagate to surface " << surface.geometryId()
+                                         << " failed: " << res.error());
+      return res.error();
+    }
+    auto &[boundParams, jacobian, pathLength] = *res;
 
-  trackStateProxy.setReferenceSurface(surface.getSharedPtr());
+    // Fill the track state
+    trackStateProxy.predicted() = std::move(boundParams.parameters());
+    if (boundParams.covariance().has_value()) {
+      trackStateProxy.predictedCovariance() =
+          std::move(*boundParams.covariance());
+    }
 
-  // Fill the track state
-  trackStateProxy.predicted() = std::move(boundParams.parameters());
-  if (boundParams.covariance().has_value()) {
-    trackStateProxy.predictedCovariance() =
-        std::move(*boundParams.covariance());
+    trackStateProxy.jacobian() = std::move(jacobian);
+    trackStateProxy.pathLength() = std::move(pathLength);
   }
-  trackStateProxy.jacobian() = std::move(jacobian);
-  trackStateProxy.pathLength() = std::move(pathLength);
 
   // We have predicted parameters, so calibrate the uncalibrated input
   // measurement
   extensions.calibrator(state.geoContext, calibrationContext, source_link,
                         trackStateProxy);
 
   // Get and set the type flags
-  auto typeFlags = trackStateProxy.typeFlags();
-  typeFlags.set(TrackStateFlag::ParameterFlag);
-  if (surface.surfaceMaterial() != nullptr) {
-    typeFlags.set(TrackStateFlag::MaterialFlag);
-  }
+  {
+    auto typeFlags = trackStateProxy.typeFlags();
+    typeFlags.set(TrackStateFlag::ParameterFlag);
+    if (surface.surfaceMaterial() != nullptr) {
+      typeFlags.set(TrackStateFlag::MaterialFlag);
+    }
 
-  // Check if the state is an outlier.
-  // If not, run Kalman update, tag it as a
-  // measurement and update the stepping state. Otherwise, just tag it as
-  // an outlier
-  if (not extensions.outlierFinder(trackStateProxy)) {
-    // Run Kalman update
-    auto updateRes = extensions.updater(state.geoContext, trackStateProxy,
-                                        state.options.direction, logger);
-    if (!updateRes.ok()) {
-      ACTS_ERROR("Update step failed: " << updateRes.error());
-      return updateRes.error();
+    // Check if the state is an outlier.
+    // If not:
+    // - run Kalman update
+    // - tag it as a measurement
+    // - update the stepping state.
+    // Else, just tag it as an outlier
+    if (not extensions.outlierFinder(trackStateProxy)) {
+      // Run Kalman update
+      auto updateRes = extensions.updater(state.geoContext, trackStateProxy,
+                                          state.options.direction, logger);
+      if (!updateRes.ok()) {
+        ACTS_ERROR("Update step failed: " << updateRes.error());
+        return updateRes.error();
+      }
+      // Set the measurement type flag
+      typeFlags.set(TrackStateFlag::MeasurementFlag);
+    } else {
+      ACTS_VERBOSE(
+          "Filtering step successful. But measurement is determined "
+          "to be an outlier. Stepping state is not updated.")
+      // Set the outlier type flag
+      typeFlags.set(TrackStateFlag::OutlierFlag);
+      trackStateProxy.shareFrom(trackStateProxy, TrackStatePropMask::Predicted,
+                                TrackStatePropMask::Filtered);
     }
-    // Set the measurement type flag
-    typeFlags.set(TrackStateFlag::MeasurementFlag);
-  } else {
-    ACTS_VERBOSE(
-        "Filtering step successful. But measurement is determined "
-        "to be an outlier. Stepping state is not updated.")
-    // Set the outlier type flag
-    typeFlags.set(TrackStateFlag::OutlierFlag);
-    trackStateProxy.shareFrom(trackStateProxy, TrackStatePropMask::Predicted,
-                              TrackStatePropMask::Filtered);
   }
 
   return trackStateProxy;
 }
 
 /// This function encapsulates what actions should be performed on a
-/// MultiTrajectory when we have no measurement
+/// MultiTrajectory when we have no measurement.
+/// If there are no source links on surface, add either a hole or passive
+/// material TrackState entry multi trajectory. No storage allocation for
+/// uncalibrated/calibrated measurement and filtered parameter
 /// @tparam propagator_state_t The propagator state type
 /// @tparam stepper_t The stepper type
 /// @param state The propagator state
@@ -127,18 +142,44 @@ auto kalmanHandleNoMeasurement(
     const Logger &logger,
     const FreeToBoundCorrection &freeToBoundCorrection = FreeToBoundCorrection(
         false)) -> Result<typename traj_t::TrackStateProxy> {
-  // No source links on surface, add either hole or passive material
-  // TrackState entry multi trajectory. No storage allocation for
-  // uncalibrated/calibrated measurement and filtered parameter
-  const auto newTrackIndex = fittedStates.addTrackState(
-      ~(TrackStatePropMask::Calibrated | TrackStatePropMask::Filtered),
-      lastTrackIndex);
+  // Add a <mask> TrackState entry multi trajectory. This allocates storage for
+  // all components, which we will set later.
+  TrackStatePropMask mask =
+      ~(TrackStatePropMask::Calibrated | TrackStatePropMask::Filtered);
+  const size_t currentTrackIndex =
+      fittedStates.addTrackState(mask, lastTrackIndex);
 
   // now get track state proxy back
-  auto trackStateProxy = fittedStates.getTrackState(newTrackIndex);
+  typename traj_t::TrackStateProxy trackStateProxy =
+      fittedStates.getTrackState(currentTrackIndex);
+
+  // Set the trackStateProxy components with the state from the ongoing
+  // propagation
+  {
+    trackStateProxy.setReferenceSurface(surface.getSharedPtr());
+    // Bind the transported state to the current surface
+    auto res = stepper.boundState(state.stepping, surface, doCovTransport,
+                                  freeToBoundCorrection);
+    if (!res.ok()) {
+      return res.error();
+    }
+    auto &[boundParams, jacobian, pathLength] = *res;
+
+    // Fill the track state
+    trackStateProxy.predicted() = std::move(boundParams.parameters());
+    if (boundParams.covariance().has_value()) {
+      trackStateProxy.predictedCovariance() =
+          std::move(*boundParams.covariance());
+    }
 
-  // Set the surface
-  trackStateProxy.setReferenceSurface(surface.getSharedPtr());
+    trackStateProxy.jacobian() = std::move(jacobian);
+    trackStateProxy.pathLength() = std::move(pathLength);
+
+    // Set the filtered parameter index to be the same with predicted
+    // parameter
+    trackStateProxy.shareFrom(trackStateProxy, TrackStatePropMask::Predicted,
+                              TrackStatePropMask::Filtered);
+  }
 
   // Set the track state flags
   auto typeFlags = trackStateProxy.typeFlags();
@@ -154,30 +195,6 @@ auto kalmanHandleNoMeasurement(
     ACTS_VERBOSE("Detected in-sensitive surface " << surface.geometryId());
   }
 
-  // Transport & bind the state to the current surface
-  auto res = stepper.boundState(state.stepping, surface, doCovTransport,
-                                freeToBoundCorrection);
-  if (!res.ok()) {
-    ACTS_ERROR("Propagate to surface " << surface.geometryId()
-                                       << " failed: " << res.error());
-    return res.error();
-  }
-  auto &[boundParams, jacobian, pathLength] = *res;
-
-  // Fill the track state
-  trackStateProxy.predicted() = std::move(boundParams.parameters());
-  if (boundParams.covariance().has_value()) {
-    trackStateProxy.predictedCovariance() =
-        std::move(*boundParams.covariance());
-  }
-  trackStateProxy.jacobian() = std::move(jacobian);
-  trackStateProxy.pathLength() = std::move(pathLength);
-
-  // Set the filtered parameter index to be the same with predicted
-  // parameter
-  trackStateProxy.shareFrom(trackStateProxy, TrackStatePropMask::Predicted,
-                            TrackStatePropMask::Filtered);
-
   return trackStateProxy;
 }