feat: Renavigation for Gen1 (#3437)

In case we loose the boundary we have to renavigate. For this purpose I introduce a loop in the `preStep` call where the first iteration has the original behavior and the second one is used in case of renavigation. Incorporates #3238 to avoid double handling of the renavigation. blocked by - #3521
acts-project · Aug 26, 2024 · cda0b99 · cda0b99
1 parent 2cfa5f7
commit cda0b99
Show file tree

Hide file tree

Showing 10 changed files with 110 additions and 60 deletions.
diff --git a/CI/physmon/reference/trackfinding_ttbar_pu200/performance_ckf.root b/CI/physmon/reference/trackfinding_ttbar_pu200/performance_ckf.root
diff --git a/CI/physmon/reference/trackfinding_ttbar_pu200/performance_ckf_ambi.root b/CI/physmon/reference/trackfinding_ttbar_pu200/performance_ckf_ambi.root
diff --git a/...smon/reference/trackfinding_ttbar_pu200/performance_vertexing_amvf_gauss_notime_hist.root b/...smon/reference/trackfinding_ttbar_pu200/performance_vertexing_amvf_gauss_notime_hist.root
diff --git a/CI/physmon/reference/trackfinding_ttbar_pu200/performance_vertexing_amvf_grid_time_hist.root b/CI/physmon/reference/trackfinding_ttbar_pu200/performance_vertexing_amvf_grid_time_hist.root
diff --git a/CI/physmon/reference/trackfinding_ttbar_pu200/tracksummary_ckf_hist.root b/CI/physmon/reference/trackfinding_ttbar_pu200/tracksummary_ckf_hist.root
diff --git a/CI/physmon/reference/trackfitting_gsf/performance_trackfitting.root b/CI/physmon/reference/trackfitting_gsf/performance_trackfitting.root
diff --git a/Core/include/Acts/Propagator/Navigator.hpp b/Core/include/Acts/Propagator/Navigator.hpp
@@ -56,9 +56,6 @@ struct NavigationOptions {
   double nearLimit = 0;
   /// The maximum distance for a surface to be considered
   double farLimit = std::numeric_limits<double>::max();
-
-  /// Force intersection with boundaries
-  bool forceIntersectBoundaries = false;
 };
 
 /// @brief Steers the propagation through the geometry by adjusting the step
@@ -184,8 +181,21 @@ class Navigator {
     bool navigationBreak = false;
     // The navigation stage (@todo: integrate break, target)
     Stage navigationStage = Stage::undefined;
-    /// Force intersection with boundaries
-    bool forceIntersectBoundaries = false;
+
+    void reset() {
+      navSurfaces.clear();
+      navSurfaceIndex = navSurfaces.size();
+      navLayers.clear();
+      navLayerIndex = navLayers.size();
+      navBoundaries.clear();
+      navBoundaryIndex = navBoundaries.size();
+
+      currentVolume = nullptr;
+      currentLayer = nullptr;
+      currentSurface = nullptr;
+
+      navigationStage = Stage::undefined;
+    }
   };
 
   /// Constructor with configuration object
@@ -373,31 +383,79 @@ class Navigator {
     // Call the navigation helper prior to actual navigation
     ACTS_VERBOSE(volInfo(state) << "Entering navigator::preStep.");
 
+    // Navigator pre step always resets the current surface
+    state.navigation.currentSurface = nullptr;
+
     // Initialize the target and target volume
     if (state.navigation.targetSurface != nullptr &&
         state.navigation.targetVolume == nullptr) {
       // Find out about the target as much as you can
       initializeTarget(state, stepper);
     }
-    // Try targeting the surfaces - then layers - then boundaries
-    if (state.navigation.navigationStage <= Stage::surfaceTarget &&
-        targetSurfaces(state, stepper)) {
-      ACTS_VERBOSE(volInfo(state) << "Target set to next surface.");
-    } else if (state.navigation.navigationStage <= Stage::layerTarget &&
-               targetLayers(state, stepper)) {
-      ACTS_VERBOSE(volInfo(state) << "Target set to next layer.");
-    } else if (targetBoundaries(state, stepper)) {
-      ACTS_VERBOSE(volInfo(state) << "Target set to next boundary.");
-    } else {
-      ACTS_VERBOSE(volInfo(state)
-                   << "No further navigation action, proceed to target.");
+
+    auto tryTargetNextSurface = [&]() {
+      // Try targeting the surfaces - then layers - then boundaries
+
+      if (state.navigation.navigationStage <= Stage::surfaceTarget &&
+          targetSurfaces(state, stepper)) {
+        ACTS_VERBOSE(volInfo(state) << "Target set to next surface.");
+        return true;
+      }
+
+      if (state.navigation.navigationStage <= Stage::layerTarget &&
+          targetLayers(state, stepper)) {
+        ACTS_VERBOSE(volInfo(state) << "Target set to next layer.");
+        return true;
+      }
+
+      if (targetBoundaries(state, stepper)) {
+        ACTS_VERBOSE(volInfo(state) << "Target set to next boundary.");
+        return true;
+      }
+
+      return false;
+    };
+
+    if (tryTargetNextSurface()) {
+      // Proceed to the next surface
+      return;
+    }
+
+    ACTS_VERBOSE(volInfo(state)
+                 << "No targets found, we got lost! Attempt renavigation.");
+
+    state.navigation.reset();
+
+    // We might have punched through a boundary and entered another volume
+    // so we have to reinitialize
+    state.navigation.currentVolume =
+        m_cfg.trackingGeometry->lowestTrackingVolume(
+            state.geoContext, stepper.position(state.stepping));
+
+    if (state.navigation.currentVolume == nullptr) {
+      ACTS_VERBOSE(volInfo(state) << "No volume found, stop navigation.");
       // Set navigation break and release the navigation step size
       state.navigation.navigationBreak = true;
       stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
+      return;
     }
 
-    // Navigator target always resets the current surface
-    state.navigation.currentSurface = nullptr;
+    state.navigation.currentLayer =
+        state.navigation.currentVolume->associatedLayer(
+            state.geoContext, stepper.position(state.stepping));
+
+    ACTS_VERBOSE(volInfo(state) << "Resolved volume and layer.");
+
+    // Rerun the targeting
+    if (tryTargetNextSurface()) {
+      return;
+    }
+
+    ACTS_VERBOSE(volInfo(state) << "No targets found again, we got "
+                                   "really lost! Stop navigation.");
+    // Set navigation break and release the navigation step size
+    state.navigation.navigationBreak = true;
+    stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
   }
 
   /// @brief Navigator post step call
@@ -504,6 +562,7 @@ class Navigator {
               << "No more volume to progress to, stopping navigation.");
           // Navigation break & release navigation stepping
           state.navigation.navigationBreak = true;
+          stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
           return;
         } else {
           ACTS_VERBOSE(volInfo(state) << "Volume updated.");
@@ -529,6 +588,7 @@ class Navigator {
       }
       // Set navigation break and release the navigation step size
       state.navigation.navigationBreak = true;
+      stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
     } else {
       ACTS_VERBOSE(volInfo(state)
                    << "Status could not be determined - good luck.");
@@ -692,8 +752,12 @@ class Navigator {
       } else {
         ACTS_VERBOSE(volInfo(state)
                      << "Last surface on layer reached, and no layer.");
-        state.navigation.navigationBreak =
-            (state.navigation.currentVolume == state.navigation.targetVolume);
+        if (state.navigation.currentVolume == state.navigation.targetVolume) {
+          ACTS_VERBOSE(volInfo(state)
+                       << "This is the target volume, stop navigation.");
+          state.navigation.navigationBreak = true;
+          stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
+        }
       }
     }
 
@@ -787,9 +851,14 @@ class Navigator {
       }
       logger().log(Logging::VERBOSE, os.str());
     }
+
     // Set the navigation break if necessary
-    state.navigation.navigationBreak =
-        (state.navigation.currentVolume == state.navigation.targetVolume);
+    if (state.navigation.currentVolume == state.navigation.targetVolume) {
+      ACTS_VERBOSE(volInfo(state)
+                   << "This is the target volume, stop navigation.");
+      state.navigation.navigationBreak = true;
+      stepper.releaseStepSize(state.stepping, ConstrainedStep::actor);
+    }
     return false;
   }
 
@@ -853,8 +922,6 @@ class Navigator {
       navOpts.nearLimit = state.options.surfaceTolerance;
       navOpts.farLimit =
           stepper.getStepSize(state.stepping, ConstrainedStep::aborter);
-      navOpts.forceIntersectBoundaries =
-          state.navigation.forceIntersectBoundaries;
 
       ACTS_VERBOSE(volInfo(state)
                    << "Try to find boundaries, we are at: "
@@ -931,17 +998,10 @@ class Navigator {
       // Increase the index to the next one
       ++state.navigation.navBoundaryIndex;
     }
-    // We have to leave the volume somehow, so try again
-    state.navigation.navBoundaries.clear();
-    ACTS_VERBOSE(volInfo(state) << "Boundary navigation lost, re-targetting.");
-    state.navigation.forceIntersectBoundaries = true;
-    if (findBoundaries()) {
-      // Resetting intersection check for boundary surfaces
-      state.navigation.forceIntersectBoundaries = false;
-      return true;
-    }
 
     // Tried our best, but couldn't do anything
+    state.navigation.navBoundaries.clear();
+    state.navigation.navBoundaryIndex = state.navigation.navBoundaries.size();
     return false;
   }
 
@@ -1038,8 +1098,14 @@ class Navigator {
                        const stepper_t& stepper) const {
     // get the layer and layer surface
     const Layer* currentLayer = state.navigation.currentLayer;
-    assert(currentLayer != nullptr && "Current layer is not set.");
+
+    if (currentLayer == nullptr) {
+      ACTS_VERBOSE(volInfo(state) << "No layer to resolve surfaces.");
+      return false;
+    }
+
     const Surface* layerSurface = &currentLayer->surfaceRepresentation();
+
     // Use navigation parameters and NavigationOptions
     NavigationOptions<Surface> navOpts;
     navOpts.resolveSensitive = m_cfg.resolveSensitive;
@@ -1060,6 +1126,7 @@ class Navigator {
         navOpts.externalSurfaces.push_back(itSurface->second);
       }
     }
+
     navOpts.nearLimit = state.options.surfaceTolerance;
     navOpts.farLimit =
         stepper.getStepSize(state.stepping, ConstrainedStep::aborter);

diff --git a/Core/src/Geometry/TrackingVolume.cpp b/Core/src/Geometry/TrackingVolume.cpp
@@ -445,24 +445,6 @@ TrackingVolume::compatibleBoundaries(const GeometryContext& gctx,
         continue;
       }
 
-      if (options.forceIntersectBoundaries) {
-        const bool coCriterion =
-            std::abs(intersection.pathLength()) < std::abs(nearLimit);
-        ACTS_VERBOSE("Forcing intersection with surface "
-                     << boundary->surfaceRepresentation().geometryId());
-        if (coCriterion) {
-          ACTS_VERBOSE("Intersection forced successfully ");
-          ACTS_VERBOSE("- intersection path length "
-                       << std::abs(intersection.pathLength())
-                       << " < overstep limit " << std::abs(nearLimit));
-          return BoundaryIntersection(intersection, boundary);
-        }
-        ACTS_VERBOSE("Can't force intersection: ");
-        ACTS_VERBOSE("- intersection path length "
-                     << std::abs(intersection.pathLength())
-                     << " > overstep limit " << std::abs(nearLimit));
-      }
-
       ACTS_VERBOSE("Check intersection with surface "
                    << boundary->surfaceRepresentation().geometryId());
       if (detail::checkPathLength(intersection.pathLength(), nearLimit,

diff --git a/Examples/Io/Root/src/RootTrackStatesWriter.cpp b/Examples/Io/Root/src/RootTrackStatesWriter.cpp
@@ -657,6 +657,7 @@ ProcessCode RootTrackStatesWriter::writeT(const AlgorithmContext& ctx,
         m_res_eT[ipar].push_back(parameters[Acts::eBoundTime] - truthTIME);
 
         // track parameters pull
+        // MARK: fpeMaskBegin(FLTDIV, 1, #2348)
         m_pull_eLOC0[ipar].push_back(
             (parameters[Acts::eBoundLoc0] - truthLOC0) /
             std::sqrt(covariance(Acts::eBoundLoc0, Acts::eBoundLoc0)));
@@ -671,12 +672,10 @@ ProcessCode RootTrackStatesWriter::writeT(const AlgorithmContext& ctx,
         m_pull_eQOP[ipar].push_back(
             (parameters[Acts::eBoundQOverP] - truthQOP) /
             std::sqrt(covariance(Acts::eBoundQOverP, Acts::eBoundQOverP)));
-        double sigmaTime =
-            std::sqrt(covariance(Acts::eBoundTime, Acts::eBoundTime));
         m_pull_eT[ipar].push_back(
-            sigmaTime == 0.0
-                ? nan
-                : (parameters[Acts::eBoundTime] - truthTIME) / sigmaTime);
+            (parameters[Acts::eBoundTime] - truthTIME) /
+            std::sqrt(covariance(Acts::eBoundTime, Acts::eBoundTime)));
+        // MARK: fpeMaskEnd(FLTDIV)
 
         if (ipar == ePredicted) {
           // local hit residual info

diff --git a/Examples/Python/tests/test_examples.py b/Examples/Python/tests/test_examples.py
@@ -1132,7 +1132,8 @@ def test_full_chain_odd_example_pythia_geant4(tmp_path):
     # This test literally only ensures that the full chain example can run without erroring out
 
     # just to make sure it can build the odd
-    detector, trackingGeometry, decorators = getOpenDataDetector()
+    with getOpenDataDetector() as (detector, trackingGeometry, decorators):
+        pass
 
     script = (
         Path(__file__).parent.parent.parent.parent
@@ -1185,7 +1186,8 @@ def test_ML_Ambiguity_Solver(tmp_path, assert_root_hash):
     assert not (tmp_path / root_file).exists()
 
     # just to make sure it can build the odd
-    detector, trackingGeometry, decorators = getOpenDataDetector()
+    with getOpenDataDetector() as (detector, trackingGeometry, decorators):
+        pass
 
     script = (
         Path(__file__).parent.parent.parent.parent