Merge branch 'ACTS-498_LineSurface_Approach' into 'master'

Resolves problem with Line surface extrapolation

Closes ACTS-498

See merge request acts/acts-core!456

Core/include/Acts/Extrapolator/Navigator.hpp

@@ -448,9 +448,11 @@ struct Navigator
           // set the iterator
           state.navigation.navSurfaceIter
               = state.navigation.navSurfaces.begin();
-          // update the navigation step size before you return
+          // Update the navigation step size before you return
+          // That's a new navigation stream, thus release the former step
           updateStep(state,
-                     state.navigation.navSurfaceIter->intersection.pathLength);
+                     state.navigation.navSurfaceIter->intersection.pathLength,
+                     true);
           return true;
         }
         return false;
@@ -665,9 +667,11 @@ struct Navigator
             return std::string("Starting from boundary surface.");
           });
         } else {
-          // update the navigation step size before you return
+          // Update the navigation step size before you return
+          // That's a new navigation stream, release the prior navigation
           updateStep(state,
-                     state.navigation.navBoundaryIter->intersection.pathLength);
+                     state.navigation.navBoundaryIter->intersection.pathLength,
+                     true);
           return true;
         }
       } else {
@@ -767,8 +771,9 @@ struct Navigator
             lastBoundary = boundarySurface;
             continue;
           } else {
-            // update the navigation step size
-            updateStep(state, boundaryIntersect.intersection.pathLength);
+            // Update the navigation step size
+            // This is a new navigation stream, release the former first
+            updateStep(state, boundaryIntersect.intersection.pathLength, true);
             return true;
           }
         } else {
@@ -843,7 +848,8 @@ struct Navigator
                  [&] { return std::string("Stepping towards first layer."); });
         // update the navigation step size before you return
         updateStep(state,
-                   state.navigation.navLayerIter->intersection.pathLength);
+                   state.navigation.navLayerIter->intersection.pathLength,
+                   true);
         return true;
       } else {
         debugLog(state, [&] {
@@ -882,6 +888,7 @@ struct Navigator
     });
     // the step size will be set to the aborter step size
     double abortStep = state.stepping.stepSize.value(cstep::aborter);
+    // Do not release the aborter step size here
     updateStep(state, abortStep);
     state.navigation.navigationBreak = true;
     return true;
@@ -995,8 +1002,8 @@ struct Navigator
           });
           ++state.navigation.navLayerIter;
         } else {
-          // update the navigation step size
-          updateStep(state, layerIntersect.intersection.pathLength);
+          // update the navigation step size, release the former first
+          updateStep(state, layerIntersect.intersection.pathLength, true);
           return true;
         }
       }
@@ -1009,7 +1016,7 @@ struct Navigator
         return std::string(
             "Done in final volume, release stepSize & proceed to target.");
       });
-      // the step size will be set to the aborter step size
+      // the step size will be set to the aborter step size, do not release
       double abortStep = state.stepping.stepSize.value(cstep::aborter);
       updateStep(state, abortStep);
       state.navigation.navigationBreak = true;
@@ -1071,8 +1078,10 @@ struct Navigator
       // set the iterator
       state.navigation.navSurfaceIter = state.navigation.navSurfaces.begin();
       // update the navigation step size before you return to the stepper
+      // This is a new navigation stream, release the former step size first
       updateStep(state,
-                 state.navigation.navSurfaceIter->intersection.pathLength);
+                 state.navigation.navSurfaceIter->intersection.pathLength,
+                 true);
       return true;
     }
     state.navigation.navSurfaceIter = state.navigation.navSurfaces.end();
@@ -1174,7 +1183,8 @@ struct Navigator
           ++state.navigation.navSurfaceIter;
           continue;
         } else {
-          updateStep(state, surfaceDistance);
+          /// Releae the former step size
+          updateStep(state, surfaceDistance, true);
           return true;
         }
       }
@@ -1200,23 +1210,25 @@ struct Navigator
   ///
   /// @param[in,out] state The state object for the step length
   /// @param[in] step the step size
+  /// @param[in] release flag steers if the step is released first
   template <typename propagator_state_t>
   void
-  updateStep(propagator_state_t& state, double step) const
+  updateStep(propagator_state_t& state, double step, bool release = false) const
   {
     /// If we have an initial step and are configured to modify it
     if (state.stepping.pathAccumulated == 0. && initialStepFactor != 1.) {
       debugLog(state, [&] {
-        return std::string("Initial step modification detected.");
+        return std::string("Initial navigation step modifiction applied.");
       });
       step *= initialStepFactor;
     }
 
     //  update the step
-    state.stepping.stepSize.update(step, cstep::actor);
+    state.stepping.stepSize.update(step, cstep::actor, release);
     debugLog(state, [&] {
       std::stringstream dstream;
-      dstream << "Navigation stepSize updated to ";
+      std::string       releaseFlag = release ? "released and " : "";
+      dstream << "Navigation stepSize " << releaseFlag << "updated to ";
       dstream << state.stepping.stepSize.toString();
       return dstream.str();
     });

Core/include/Acts/Propagator/detail/ConstrainedStep.hpp

@@ -38,17 +38,22 @@ namespace detail {
     /// The Navigation direction
     NavigationDirection direction = forward;
 
-    /// update the step size of a certain type
-    /// - for accuracy and navigation that can go either way
-    /// - for aborters it can only get (direction)*smaller
+    /// Update the step size of a certain type
+    ///
+    /// Only navigation and target abortion step size
+    /// updates may change the sign due to overstepping
+    ///
     /// @param value is the new value to be updated
     /// @param type is the constraint type
     void
-    update(const double& value, Type type)
+    update(const double& value, Type type, bool releaseStep = false)
     {
-      if (type != aborter || (direction * values[type] > direction * value)) {
-        values[type] = value;
+      if (releaseStep) {
+        release(type);
       }
+      // The check the current value and set it if appropriate
+      double cValue = values[type];
+      values[type]  = cValue * cValue < value * value ? cValue : value;
     }
 
     /// release a certain constraint value

Core/include/Acts/Propagator/detail/StandardAbortConditions.hpp

@@ -160,7 +160,6 @@ namespace detail {
         return true;
       }
       // calculate the distance to the surface
-      // @todo: add corrector
       const double tolerance = state.options.targetTolerance;
       const auto   iestimate
           = state.navigation.targetSurface->intersectionEstimate(

Tests/Core/Extrapolator/MaterialCollectionTests.cpp

@@ -70,8 +70,8 @@ namespace Test {
   StraightLinePropagator slpropagator(std::move(slstepper),
                                       std::move(navigatorSL));
 
-  const int ntests           = 10;  // test with 0.4 lower boundary - 952;
-  const int skip             = 0;   // test with 0.4 lower boundary - 951;
+  const int ntests           = 100;
+  const int skip             = 0;
   bool      debugModeFwd     = false;
   bool      debugModeBwd     = false;
   bool      debugModeFwdStep = false;

Tests/Core/Propagator/ConstrainedStepTests.cpp

@@ -53,8 +53,12 @@ namespace Test {
     // now we update the actor to smaller
     stepSize_p.update(0.05, cstep::actor);
     BOOST_CHECK(stepSize_p == 0.05);
-    // we increase the actor and accuracy is smaller again
-    stepSize_p.update(0.15, cstep::actor);
+    // we increase the actor, but do not release the step size
+    stepSize_p.update(0.15, cstep::actor, false);
+    BOOST_CHECK(stepSize_p == 0.05);
+    // we increase the actor, but now DO release the step size
+    // it falls back to the accuracy
+    stepSize_p.update(0.15, cstep::actor, true);
     BOOST_CHECK(stepSize_p == 0.1);
 
     // now set two and update them
@@ -86,8 +90,11 @@ namespace Test {
     // now we update the actor to smaller
     stepSize_n.update(-0.05, cstep::actor);
     BOOST_CHECK(stepSize_n == -0.05);
-    // we increase the actor and accuracy is smaller again
-    stepSize_n.update(-0.15, cstep::actor);
+    // we increase the actor and accuracy is smaller again, without reset
+    stepSize_n.update(-0.15, cstep::actor, false);
+    BOOST_CHECK(stepSize_n == -0.05);
+    // we increase the actor and accuracy is smaller again, with reset
+    stepSize_n.update(-0.15, cstep::actor, true);
     BOOST_CHECK(stepSize_n == -0.1);
 
     // now set two and update them

Tests/Integration/PropagationTestBase.hpp

@@ -332,6 +332,7 @@ BOOST_DATA_TEST_CASE(
   BOOST_CHECK(e_at_disc.first.isApprox(a_at_disc.first, 1 * units::_um));
   BOOST_CHECK(e_at_disc.first.isApprox(w_at_disc.first, 1 * units::_um));
 }
+
 /// test consistency of propagators to a line
 BOOST_DATA_TEST_CASE(
     propagation_to_line_,
@@ -432,6 +433,7 @@ BOOST_DATA_TEST_CASE(
   BOOST_CHECK(e_at_line.first.isApprox(a_at_line.first, 1 * units::_um));
   BOOST_CHECK(e_at_line.first.isApprox(w_at_line.first, 1 * units::_um));
 }
+
 /// test correct covariance transport for curvilinear parameters
 /// this test only works within the
 /// s_curvilinearProjTolerance (in: Definitions.hpp)

Tests/Integration/PropagationTestHelper.hpp

@@ -365,7 +365,12 @@ namespace IntegrationTest {
     Surface_type endSurface(seTransform, nullptr);
     // Increase the path limit - to be safe hitting the surface
     options.pathLimit *= 2;
-    options.maxStepSize *= 0.9;
+
+    if (debug) {
+      std::cout << ">>> Path limit for this propgation is set to: "
+                << options.pathLimit << std::endl;
+    }
+
     const auto  result = propagator.propagate(start, endSurface, options);
     const auto& tp     = result.endParameters;
     // check the result for nullptr

Tests/Integration/PropagationTests.cpp

@@ -52,7 +52,7 @@ namespace IntegrationTest {
       = PropagatorWrapper<std::shared_ptr<PropagationEngine_type>>;
 
   // number of tests
-  const int  ntests = 1;
+  const int  ntests = 100;
   const int  skip   = 0;
   const bool covtpr = true;
   const bool debug  = false;