From 7251c28f7692b402342693164087df3ec1fe512f Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 20 Jul 2023 15:10:30 +0200 Subject: [PATCH] refactor: Remove direction from constrained step (#2073) With this PR I want to remove the propagation direction sign inside the `ConstrainedStep` and put it into the stepper as it feels like additional burden and logic that should not be handled by the `ConstrainedStep`. This will not change anything about the overstepping mechanics. --- .../include/Acts/Navigation/NextNavigator.hpp | 1 - Core/include/Acts/Propagator/AtlasStepper.hpp | 7 ++-- .../Acts/Propagator/ConstrainedStep.hpp | 34 ++++++---------- .../Acts/Propagator/DirectNavigator.hpp | 1 - Core/include/Acts/Propagator/EigenStepper.hpp | 3 +- Core/include/Acts/Propagator/EigenStepper.ipp | 4 +- .../Acts/Propagator/MultiEigenStepperLoop.hpp | 1 + Core/include/Acts/Propagator/Propagator.hpp | 4 +- .../Acts/Propagator/StandardAborters.hpp | 8 ++-- .../Acts/Propagator/StraightLineStepper.hpp | 4 +- .../Acts/Propagator/detail/SteppingHelper.hpp | 6 +-- .../Acts/Propagator/detail/SteppingLogger.hpp | 3 ++ .../CombinatorialKalmanFilter.hpp | 3 +- .../Acts/TrackFitting/KalmanFitter.hpp | 7 +--- Core/src/Geometry/Layer.cpp | 3 -- Core/src/Geometry/TrackingVolume.cpp | 31 ++++---------- Examples/Python/tests/root_file_hashes.txt | 8 ++-- .../Core/Propagator/AtlasStepperTests.cpp | 25 ++++++------ .../UnitTests/Core/Propagator/CMakeLists.txt | 2 +- .../Core/Propagator/ConstrainedStepTests.cpp | 40 ------------------- ...StepperTests.cpp => EigenStepperTests.cpp} | 10 ++--- .../Propagator/MaterialCollectionTests.cpp | 4 +- .../Core/Propagator/PropagatorTests.cpp | 3 +- .../Propagator/StraightLineStepperTests.cpp | 16 ++++---- 24 files changed, 79 insertions(+), 149 deletions(-) rename Tests/UnitTests/Core/Propagator/{StepperTests.cpp => EigenStepperTests.cpp} (99%) diff --git a/Core/include/Acts/Navigation/NextNavigator.hpp b/Core/include/Acts/Navigation/NextNavigator.hpp index 78d982742bb..ce774dedd02 100644 --- a/Core/include/Acts/Navigation/NextNavigator.hpp +++ b/Core/include/Acts/Navigation/NextNavigator.hpp @@ -16,7 +16,6 @@ #include "Acts/Geometry/GeometryIdentifier.hpp" #include "Acts/Geometry/Layer.hpp" #include "Acts/Navigation/NavigationState.hpp" -#include "Acts/Propagator/ConstrainedStep.hpp" #include "Acts/Propagator/Propagator.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Utilities/Logger.hpp" diff --git a/Core/include/Acts/Propagator/AtlasStepper.hpp b/Core/include/Acts/Propagator/AtlasStepper.hpp index b27f53df873..b74535b56d5 100644 --- a/Core/include/Acts/Propagator/AtlasStepper.hpp +++ b/Core/include/Acts/Propagator/AtlasStepper.hpp @@ -63,7 +63,7 @@ class AtlasStepper { double stolerance = s_onSurfaceTolerance) : navDir(ndir), field(0., 0., 0.), - stepSize(ndir * std::abs(ssize)), + stepSize(ssize), tolerance(stolerance), fieldCache(std::move(fieldCacheIn)), geoContext(gctx) { @@ -1113,7 +1113,7 @@ class AtlasStepper { Result step(propagator_state_t& state, const navigator_t& /*navigator*/) const { // we use h for keeping the nominclature with the original atlas code - auto h = state.stepping.stepSize.value(); + auto h = state.stepping.stepSize.value() * state.stepping.navDir; bool Jac = state.stepping.useJacobian; double* R = &(state.stepping.pVector[0]); // Coordinates @@ -1224,7 +1224,8 @@ class AtlasStepper { std::abs((C1 + C6) - (C3 + C4))); if (std::abs(EST) > std::abs(state.options.tolerance)) { h = h * .5; - state.stepping.stepSize.setValue(h); + // neutralize the sign of h again + state.stepping.stepSize.setValue(h * state.stepping.navDir); // dltm = 0.; nStepTrials++; continue; diff --git a/Core/include/Acts/Propagator/ConstrainedStep.hpp b/Core/include/Acts/Propagator/ConstrainedStep.hpp index 8c0e5d5e28b..d8012207e6c 100644 --- a/Core/include/Acts/Propagator/ConstrainedStep.hpp +++ b/Core/include/Acts/Propagator/ConstrainedStep.hpp @@ -9,7 +9,6 @@ #pragma once #include "Acts/Definitions/Algebra.hpp" -#include "Acts/Definitions/Direction.hpp" #include #include @@ -21,7 +20,11 @@ namespace Acts { -/// A constrained step class for the steppers +/// A constrained step class for the steppers. +/// +/// This class is symmetrical for forward and backward propagation. The sign of +/// the propagation direction should not enter here but rather be applied the +/// step is actually taken. /// /// As simple as this class looks it hides a few very important details: /// - Overstepping handling. The step size sign will flip if we happened to pass @@ -30,9 +33,8 @@ namespace Acts { /// order to converge on a target. /// /// Because of the points mentioned above, the update function will always -/// prefer step sizes that point opposite the navigation direction. A side -/// effect of this is that we will propagate in the opposite direction if the -/// target is "behind us". +/// prefer negative step sizes. A side effect of this is that we will propagate +/// in the opposite direction if the target is "behind us". /// /// The hierarchy is: /// - Overstepping resolution / backpropagation @@ -56,12 +58,8 @@ class ConstrainedStep { constexpr ConstrainedStep() = default; /// constructor from Scalar - /// navigation direction is inferred by the sign of the step size /// @param value is the user given initial value - constexpr explicit ConstrainedStep(Scalar value) { - m_values[user] = std::abs(value); - m_direction = Direction::fromScalar(value); - } + constexpr explicit ConstrainedStep(Scalar value) { m_values[user] = value; } /// set accuracy by one Scalar /// @@ -71,7 +69,7 @@ class ConstrainedStep { /// @param value is the new accuracy value constexpr void setValue(Scalar value) { /// set the accuracy value - m_values[accuracy] = value * m_direction; + m_values[accuracy] = value; } /// returns the min step size @@ -80,9 +78,7 @@ class ConstrainedStep { /// Access a specific value /// /// @param type is the requested parameter type - constexpr Scalar value(Type type) const { - return m_values[type] * m_direction; - } + constexpr Scalar value(Type type) const { return m_values[type]; } /// Access the currently leading type constexpr Type currentType() const { @@ -109,14 +105,14 @@ class ConstrainedStep { } // check the current value and set it if appropriate // this will also allow signed values due to overstepping - if (std::abs(value) <= std::abs(m_values[type])) { - m_values[type] = value * m_direction; + if (std::abs(value) < std::abs(m_values[type])) { + m_values[type] = value; } } constexpr void scale(Scalar factor) { assert(factor > 0 && "ConstrainedStep scale factor was zero or negative."); - m_values[accuracy] = value() * factor * m_direction; + m_values[accuracy] = value() * factor; } std::ostream& toStream(std::ostream& os) const { @@ -154,11 +150,7 @@ class ConstrainedStep { inline static constexpr auto kNotSet = std::numeric_limits::max(); /// the step size tuple - /// all values point in the `m_direction` std::array m_values = {kNotSet, kNotSet, kNotSet, kNotSet}; - /// the navigation direction - /// the direction is invariant after initialization - Direction m_direction = Direction::Forward; }; inline std::ostream& operator<<(std::ostream& os, const ConstrainedStep& step) { diff --git a/Core/include/Acts/Propagator/DirectNavigator.hpp b/Core/include/Acts/Propagator/DirectNavigator.hpp index 7f593c48de6..8b91dc67336 100644 --- a/Core/include/Acts/Propagator/DirectNavigator.hpp +++ b/Core/include/Acts/Propagator/DirectNavigator.hpp @@ -12,7 +12,6 @@ #include "Acts/Geometry/Layer.hpp" #include "Acts/Geometry/TrackingGeometry.hpp" #include "Acts/Geometry/TrackingVolume.hpp" -#include "Acts/Propagator/ConstrainedStep.hpp" #include "Acts/Propagator/Propagator.hpp" #include "Acts/Surfaces/Surface.hpp" diff --git a/Core/include/Acts/Propagator/EigenStepper.hpp b/Core/include/Acts/Propagator/EigenStepper.hpp index ddff9ca821d..c7eae5c5342 100644 --- a/Core/include/Acts/Propagator/EigenStepper.hpp +++ b/Core/include/Acts/Propagator/EigenStepper.hpp @@ -16,6 +16,7 @@ #include "Acts/Definitions/Units.hpp" #include "Acts/EventData/TrackParameters.hpp" #include "Acts/MagneticField/MagneticFieldProvider.hpp" +#include "Acts/Propagator/ConstrainedStep.hpp" #include "Acts/Propagator/DefaultExtension.hpp" #include "Acts/Propagator/DenseEnvironmentExtension.hpp" #include "Acts/Propagator/EigenStepperError.hpp" @@ -80,7 +81,7 @@ class EigenStepper { double ssize = std::numeric_limits::max()) : absCharge(std::abs(par.charge())), navDir(ndir), - stepSize(ndir * std::abs(ssize)), + stepSize(ssize), fieldCache(std::move(fieldCacheIn)), geoContext(gctx) { pars.template segment<3>(eFreePos0) = par.position(gctx); diff --git a/Core/include/Acts/Propagator/EigenStepper.ipp b/Core/include/Acts/Propagator/EigenStepper.ipp index 5ddeeff3eb5..53a8338a0f9 100644 --- a/Core/include/Acts/Propagator/EigenStepper.ipp +++ b/Core/include/Acts/Propagator/EigenStepper.ipp @@ -147,7 +147,7 @@ Acts::Result Acts::EigenStepper::step( constexpr auto success = &Result::success; constexpr auto failure = &Result::failure; - const double h = step.value(); + const double h = step.value() * state.stepping.navDir; // State the square and half of the step size h2 = h * h; half_h = h * 0.5; @@ -230,7 +230,7 @@ Acts::Result Acts::EigenStepper::step( } // use the adjusted step size - const double h = state.stepping.stepSize.value(); + const double h = state.stepping.stepSize.value() * state.stepping.navDir; // When doing error propagation, update the associated Jacobian matrix if (state.stepping.covTransport) { diff --git a/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp b/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp index d53c278e2e0..55131f4d93f 100644 --- a/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp +++ b/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp @@ -18,6 +18,7 @@ #include "Acts/EventData/TrackParameters.hpp" #include "Acts/EventData/detail/CorrectedTransformationFreeToBound.hpp" #include "Acts/MagneticField/MagneticFieldProvider.hpp" +#include "Acts/Propagator/ConstrainedStep.hpp" #include "Acts/Propagator/EigenStepper.hpp" #include "Acts/Propagator/EigenStepperError.hpp" #include "Acts/Propagator/Propagator.hpp" diff --git a/Core/include/Acts/Propagator/Propagator.hpp b/Core/include/Acts/Propagator/Propagator.hpp index 4404c5a0913..c5b894d7aee 100644 --- a/Core/include/Acts/Propagator/Propagator.hpp +++ b/Core/include/Acts/Propagator/Propagator.hpp @@ -136,7 +136,7 @@ struct PropagatorOptions : public PropagatorPlainOptions { eoptions.mass = mass; eoptions.maxSteps = maxSteps; eoptions.maxRungeKuttaStepTrials = maxRungeKuttaStepTrials; - eoptions.maxStepSize = direction * std::abs(maxStepSize); + eoptions.maxStepSize = maxStepSize; eoptions.targetTolerance = targetTolerance; eoptions.pathLimit = direction * std::abs(pathLimit); eoptions.loopProtection = loopProtection; @@ -162,7 +162,7 @@ struct PropagatorOptions : public PropagatorPlainOptions { mass = pOptions.mass; maxSteps = pOptions.maxSteps; maxRungeKuttaStepTrials = pOptions.maxRungeKuttaStepTrials; - maxStepSize = direction * std::abs(pOptions.maxStepSize); + maxStepSize = pOptions.maxStepSize; targetTolerance = pOptions.targetTolerance; pathLimit = direction * std::abs(pOptions.pathLimit); loopProtection = pOptions.loopProtection; diff --git a/Core/include/Acts/Propagator/StandardAborters.hpp b/Core/include/Acts/Propagator/StandardAborters.hpp index b04cf66d532..8d6a6137cc8 100644 --- a/Core/include/Acts/Propagator/StandardAborters.hpp +++ b/Core/include/Acts/Propagator/StandardAborters.hpp @@ -63,8 +63,8 @@ struct PathLimitReached { return true; } // Check if the maximum allowed step size has to be updated - double distance = state.stepping.navDir * std::abs(internalLimit) - - state.stepping.pathAccumulated; + double distance = + std::abs(internalLimit) - std::abs(state.stepping.pathAccumulated); double tolerance = state.options.targetTolerance; stepper.setStepSize(state.stepping, distance, ConstrainedStep::aborter, false); @@ -169,8 +169,8 @@ struct SurfaceReached { // Update the distance to the alternative solution distance = sIntersection.alternative.pathLength; } - stepper.setStepSize(state.stepping, state.stepping.navDir * distance, - ConstrainedStep::aborter, false); + stepper.setStepSize(state.stepping, distance, ConstrainedStep::aborter, + false); ACTS_VERBOSE("Target: 0 | " << "Target stepSize (surface) updated to " diff --git a/Core/include/Acts/Propagator/StraightLineStepper.hpp b/Core/include/Acts/Propagator/StraightLineStepper.hpp index d9a01fcb979..e779dc41b1c 100644 --- a/Core/include/Acts/Propagator/StraightLineStepper.hpp +++ b/Core/include/Acts/Propagator/StraightLineStepper.hpp @@ -79,7 +79,7 @@ class StraightLineStepper { double stolerance = s_onSurfaceTolerance) : absCharge(std::abs(par.charge())), navDir(ndir), - stepSize(ndir * std::abs(ssize)), + stepSize(ssize), tolerance(stolerance), geoContext(gctx) { (void)mctx; @@ -395,7 +395,7 @@ class StraightLineStepper { Result step(propagator_state_t& state, const navigator_t& /*navigator*/) const { // use the adjusted step size - const auto h = state.stepping.stepSize.value(); + const auto h = state.stepping.stepSize.value() * state.stepping.navDir; const double p = absoluteMomentum(state.stepping); // time propagates along distance as 1/b = sqrt(1 + m²/p²) const auto dtds = std::hypot(1., state.options.mass / p); diff --git a/Core/include/Acts/Propagator/detail/SteppingHelper.hpp b/Core/include/Acts/Propagator/detail/SteppingHelper.hpp index 53d95cd995d..33d1c30ad48 100644 --- a/Core/include/Acts/Propagator/detail/SteppingHelper.hpp +++ b/Core/include/Acts/Propagator/detail/SteppingHelper.hpp @@ -57,8 +57,7 @@ Acts::Intersection3D::Status updateSingleSurfaceStatus( if (detail::checkIntersection(sIntersection.intersection, pLimit, oLimit, surfaceTolerance, logger)) { ACTS_VERBOSE("Surface is reachable"); - stepper.setStepSize(state, - state.navDir * sIntersection.intersection.pathLength); + stepper.setStepSize(state, sIntersection.intersection.pathLength); return Intersection3D::Status::reachable; } @@ -66,8 +65,7 @@ Acts::Intersection3D::Status updateSingleSurfaceStatus( detail::checkIntersection(sIntersection.alternative, pLimit, oLimit, surfaceTolerance, logger)) { ACTS_VERBOSE("Surface is reachable"); - stepper.setStepSize(state, - state.navDir * sIntersection.alternative.pathLength); + stepper.setStepSize(state, sIntersection.alternative.pathLength); return Intersection3D::Status::reachable; } } diff --git a/Core/include/Acts/Propagator/detail/SteppingLogger.hpp b/Core/include/Acts/Propagator/detail/SteppingLogger.hpp index b726d61aa67..42ba231d807 100644 --- a/Core/include/Acts/Propagator/detail/SteppingLogger.hpp +++ b/Core/include/Acts/Propagator/detail/SteppingLogger.hpp @@ -10,6 +10,7 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/Definitions/Common.hpp" +#include "Acts/Definitions/Direction.hpp" #include "Acts/Geometry/GeometryIdentifier.hpp" #include "Acts/Propagator/ConstrainedStep.hpp" #include "Acts/Utilities/Logger.hpp" @@ -28,6 +29,7 @@ namespace detail { /// @brief the step information for struct Step { ConstrainedStep stepSize; + Direction navDir; Vector3 position = Vector3(0., 0., 0.); Vector3 momentum = Vector3(0., 0., 0.); std::shared_ptr surface = nullptr; @@ -70,6 +72,7 @@ struct SteppingLogger { // record the propagation state Step step; step.stepSize = state.stepping.stepSize; + step.navDir = state.stepping.navDir; step.position = stepper.position(state.stepping); step.momentum = stepper.momentum(state.stepping); diff --git a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp index fc1f32097ad..4dbed25de7d 100644 --- a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp +++ b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp @@ -1226,8 +1226,7 @@ class CombinatorialKalmanFilter { state.stepping.jacTransport = FreeMatrix::Identity(); state.stepping.derivative = FreeVector::Zero(); // Reset the step size - state.stepping.stepSize = ConstrainedStep( - state.stepping.navDir * std::abs(state.options.maxStepSize)); + state.stepping.stepSize = ConstrainedStep(state.options.maxStepSize); // Set accumulatd path to zero before targeting surface state.stepping.pathAccumulated = 0.; diff --git a/Core/include/Acts/TrackFitting/KalmanFitter.hpp b/Core/include/Acts/TrackFitting/KalmanFitter.hpp index f5ce5306332..29effee9f69 100644 --- a/Core/include/Acts/TrackFitting/KalmanFitter.hpp +++ b/Core/include/Acts/TrackFitting/KalmanFitter.hpp @@ -515,9 +515,7 @@ class KalmanFitter { state.stepping.navDir = state.stepping.navDir.invert(); // Reset propagator options - state.options.maxStepSize = - state.stepping.navDir * std::abs(state.options.maxStepSize); - // Not sure if reset of pathLimit during propagation makes any sense + // TODO Not sure if reset of pathLimit during propagation makes any sense state.options.pathLimit = state.stepping.navDir * std::abs(state.options.pathLimit); @@ -978,8 +976,7 @@ class KalmanFitter { state.stepping.navDir = state.stepping.navDir.invert(); } // Reset the step size - state.stepping.stepSize = ConstrainedStep( - state.stepping.navDir * std::abs(state.options.maxStepSize)); + state.stepping.stepSize = ConstrainedStep(state.options.maxStepSize); // Set accumulatd path to zero before targeting surface state.stepping.pathAccumulated = 0.; diff --git a/Core/src/Geometry/Layer.cpp b/Core/src/Geometry/Layer.cpp index db1fc6fbd87..68fbdf799b0 100644 --- a/Core/src/Geometry/Layer.cpp +++ b/Core/src/Geometry/Layer.cpp @@ -289,15 +289,12 @@ Acts::SurfaceIntersection Acts::Layer::surfaceOnApproach( if (detail::checkIntersection(isection.intersection, pLimit, oLimit, s_onSurfaceTolerance)) { - isection.intersection.pathLength *= options.navDir; return isection; } if (isection.alternative and detail::checkIntersection(isection.alternative, pLimit, oLimit, s_onSurfaceTolerance)) { - // Set the right sign for the path length - isection.alternative.pathLength *= options.navDir; return SurfaceIntersection(isection.alternative, isection.object); } diff --git a/Core/src/Geometry/TrackingVolume.cpp b/Core/src/Geometry/TrackingVolume.cpp index 63b1355c977..7f52c3097f7 100644 --- a/Core/src/Geometry/TrackingVolume.cpp +++ b/Core/src/Geometry/TrackingVolume.cpp @@ -489,7 +489,7 @@ Acts::TrackingVolume::compatibleBoundaries( } if (options.forceIntersectBoundaries and - sIntersection.intersection.pathLength * options.navDir > 0) { + sIntersection.intersection.pathLength > 0) { const bool coCriterion = std::abs(sIntersection.intersection.pathLength) < std::abs(oLimit); ACTS_VERBOSE("Forcing intersection with surface " @@ -499,7 +499,6 @@ Acts::TrackingVolume::compatibleBoundaries( ACTS_VERBOSE("- intersection path length " << std::abs(sIntersection.intersection.pathLength) << " < overstep limit " << std::abs(oLimit)); - sIntersection.intersection.pathLength *= options.navDir; return BoundaryIntersection(sIntersection.intersection, bSurface, sIntersection.object); } @@ -515,7 +514,6 @@ Acts::TrackingVolume::compatibleBoundaries( decltype(logger)>( sIntersection.intersection, pLimit, oLimit, s_onSurfaceTolerance, logger)) { - sIntersection.intersection.pathLength *= options.navDir; return BoundaryIntersection(sIntersection.intersection, bSurface, sIntersection.object); } @@ -526,7 +524,6 @@ Acts::TrackingVolume::compatibleBoundaries( decltype(logger)>( sIntersection.alternative, pLimit, oLimit, s_onSurfaceTolerance, logger)) { - sIntersection.alternative.pathLength *= options.navDir; return BoundaryIntersection(sIntersection.alternative, bSurface, sIntersection.object); ; @@ -595,20 +592,11 @@ Acts::TrackingVolume::compatibleBoundaries( return false; }; - // Sort them accordingly to the navigation direction - if (options.navDir == Direction::Forward) { - std::sort(bIntersections.begin(), bIntersections.end(), - [&](const auto& a, const auto& b) { - return comparator(a.intersection.pathLength, - b.intersection.pathLength); - }); - } else { - std::sort(bIntersections.begin(), bIntersections.end(), - [&](const auto& a, const auto& b) { - return comparator(-a.intersection.pathLength, - -b.intersection.pathLength); - }); - } + std::sort(bIntersections.begin(), bIntersections.end(), + [&](const auto& a, const auto& b) { + return comparator(a.intersection.pathLength, + b.intersection.pathLength); + }); return bIntersections; } @@ -652,12 +640,7 @@ Acts::TrackingVolume::compatibleLayers( ? nullptr : tLayer->nextLayer(gctx, position, options.navDir * direction); } - // sort them accordingly to the navigation direction - if (options.navDir == Direction::Forward) { - std::sort(lIntersections.begin(), lIntersections.end()); - } else { - std::sort(lIntersections.begin(), lIntersections.end(), std::greater<>()); - } + std::sort(lIntersections.begin(), lIntersections.end()); } // and return return lIntersections; diff --git a/Examples/Python/tests/root_file_hashes.txt b/Examples/Python/tests/root_file_hashes.txt index f3f8ad65001..87db6ce4483 100644 --- a/Examples/Python/tests/root_file_hashes.txt +++ b/Examples/Python/tests/root_file_hashes.txt @@ -25,19 +25,19 @@ test_material_recording__geant4_material_tracks.root: e411152d370775463c22b19a35 test_truth_tracking_kalman[generic-0.0]__trackstates_fitter.root: 7e933e0219728ad8c139c2c8d0a7b09d133108a2c3053b0e1972189e34534592 test_truth_tracking_kalman[generic-0.0]__tracksummary_fitter.root: db6695050d0290e43b3579b8b6f66f013cbc33548c2bfb5ae8f4ff8306c6e400 test_truth_tracking_kalman[generic-0.0]__performance_track_finder.root: 7fc6f717723c9eddcbf44820b384b373cee6f04b72f79902f938f35e3ff9b470 -test_truth_tracking_kalman[generic-1000.0]__trackstates_fitter.root: 2298c2ae839e8208bc640e7208d2dc406f3316042c50f23ff0a65e4652a0bc2c +test_truth_tracking_kalman[generic-1000.0]__trackstates_fitter.root: 6c0313a50148aa71e20d817642cd2dffc85e467915298f00d75bef33ad83cf3c test_truth_tracking_kalman[generic-1000.0]__tracksummary_fitter.root: 89ea90d7dc0d36e7d7ab2db0f0dfe1c5b14e04533a969684e59195bed037777a test_truth_tracking_kalman[generic-1000.0]__performance_track_finder.root: 7fc6f717723c9eddcbf44820b384b373cee6f04b72f79902f938f35e3ff9b470 test_truth_tracking_kalman[odd-0.0]__trackstates_fitter.root: 4093d18a84cd01ddec712adadcf8f271e7dd1fa473a734f44b80963477fc4f54 test_truth_tracking_kalman[odd-0.0]__tracksummary_fitter.root: a515049efc3c1aab2147646dbe0f0977bd72af139fccab054fef3774af84ba90 test_truth_tracking_kalman[odd-0.0]__performance_track_finder.root: 39aec6316cceb90e314e16b02947faa691c18f57c3a851a25e547a8fc05a4593 -test_truth_tracking_kalman[odd-1000.0]__trackstates_fitter.root: 11f863960ba24d32832a620e3a4447fd8efceda09b620bdaef35a5fcc54a5676 +test_truth_tracking_kalman[odd-1000.0]__trackstates_fitter.root: 0fcf2020a10ddd7030cd1b58ef7affd60121b83eabb92e9547818e93bab9c777 test_truth_tracking_kalman[odd-1000.0]__tracksummary_fitter.root: fcad595e4062157fa6f2c871ca31d203aa85dced26ed92dc286b61c571314123 test_truth_tracking_kalman[odd-1000.0]__performance_track_finder.root: 39aec6316cceb90e314e16b02947faa691c18f57c3a851a25e547a8fc05a4593 test_truth_tracking_gsf[generic]__trackstates_gsf.root: 787294f42fadbd14827ae47133ba90d657999fa815df3fa01e3ddc3c0709d880 test_truth_tracking_gsf[generic]__tracksummary_gsf.root: 3229442f127c05f9f1d9111c85df65147a5ef285ff473f102da4221e6354bd73 -test_truth_tracking_gsf[odd]__trackstates_gsf.root: 8ea8bdeda5ba2d4fbedd5b25b1d9f01a47980f9ae5398ad157b40ec5fa070cda -test_truth_tracking_gsf[odd]__tracksummary_gsf.root: 2e57260b989a973153050d8931d05451b7b5f7aabbfa56fa40d964734a7f5d82 +test_truth_tracking_gsf[odd]__trackstates_gsf.root: 0c6b46ee55e992c0846abea69d69205cbec358aa48330396ce83955bb2153177 +test_truth_tracking_gsf[odd]__tracksummary_gsf.root: 6ff39a138089c9b246958df1052b73cd4afd453f954ba568336eb988e826fcb7 test_particle_gun__particles.root: 8549ba6e20338004ab8ba299fc65e1ee5071985b46df8f77f887cb6fef56a8ec test_material_mapping__material-map_tracks.root: 4e1c866038f0c06b099aa74fd01c3d875f07b89f54898f90debd9b558d8e4025 test_material_mapping__propagation-material.root: 646b8e2bbacec40d0bc4132236f9ab3f03b088e656e6e9b80c47ae03eaf6eab5 diff --git a/Tests/UnitTests/Core/Propagator/AtlasStepperTests.cpp b/Tests/UnitTests/Core/Propagator/AtlasStepperTests.cpp index db56cf3db59..58b995ab371 100644 --- a/Tests/UnitTests/Core/Propagator/AtlasStepperTests.cpp +++ b/Tests/UnitTests/Core/Propagator/AtlasStepperTests.cpp @@ -118,7 +118,7 @@ BOOST_AUTO_TEST_CASE(ConstructState) { BOOST_CHECK_EQUAL(state.pVector[7], charge / absMom); BOOST_CHECK_EQUAL(state.navDir, navDir); BOOST_CHECK_EQUAL(state.pathAccumulated, 0.); - BOOST_CHECK_EQUAL(state.stepSize.value(), navDir * stepSize); + BOOST_CHECK_EQUAL(state.stepSize.value(), stepSize); BOOST_CHECK_EQUAL(state.previousStepSize, 0.); BOOST_CHECK_EQUAL(state.tolerance, tolerance); } @@ -142,7 +142,7 @@ BOOST_AUTO_TEST_CASE(ConstructStateWithCovariance) { BOOST_CHECK_EQUAL(state.pVector[7], charge / absMom); BOOST_CHECK_EQUAL(state.navDir, navDir); BOOST_CHECK_EQUAL(state.pathAccumulated, 0.); - BOOST_CHECK_EQUAL(state.stepSize.value(), navDir * stepSize); + BOOST_CHECK_EQUAL(state.stepSize.value(), stepSize); BOOST_CHECK_EQUAL(state.previousStepSize, 0.); BOOST_CHECK_EQUAL(state.tolerance, tolerance); } @@ -286,8 +286,8 @@ BOOST_AUTO_TEST_CASE(Step) { // extract the actual step size auto h = res.value(); - BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), navDir * stepSize); - BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), h); + BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), stepSize); + BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), h * navDir); // check that the position has moved auto deltaPos = (stepper.position(state.stepping) - pos).eval(); @@ -319,8 +319,8 @@ BOOST_AUTO_TEST_CASE(StepWithCovariance) { // extract the actual step size auto h = res.value(); - BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), navDir * stepSize); - BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), h); + BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), stepSize); + BOOST_CHECK_EQUAL(state.stepping.stepSize.value(), h * navDir); // check that the position has moved auto deltaPos = (stepper.position(state.stepping) - pos).eval(); @@ -551,12 +551,12 @@ BOOST_AUTO_TEST_CASE(StepSize) { // TODO figure out why this fails and what it should be // BOOST_CHECK_EQUAL(stepper.overstepLimit(state), tolerance); - stepper.setStepSize(state, 5_cm); - BOOST_CHECK_EQUAL(state.previousStepSize, navDir * stepSize); - BOOST_CHECK_EQUAL(state.stepSize.value(), 5_cm); + stepper.setStepSize(state, -5_cm); + BOOST_CHECK_EQUAL(state.previousStepSize, stepSize); + BOOST_CHECK_EQUAL(state.stepSize.value(), -5_cm); stepper.releaseStepSize(state); - BOOST_CHECK_EQUAL(state.stepSize.value(), navDir * stepSize); + BOOST_CHECK_EQUAL(state.stepSize.value(), stepSize); } // test step size modification with target surfaces @@ -572,8 +572,7 @@ BOOST_AUTO_TEST_CASE(StepSizeSurface) { pos + navDir * distance * unitDir, unitDir); stepper.updateSurfaceStatus(state, *target, BoundaryCheck(false)); - BOOST_CHECK_EQUAL(state.stepSize.value(ConstrainedStep::actor), - navDir * distance); + BOOST_CHECK_EQUAL(state.stepSize.value(ConstrainedStep::actor), distance); // test the step size modification in the context of a surface stepper.updateStepSize( @@ -590,7 +589,7 @@ BOOST_AUTO_TEST_CASE(StepSizeSurface) { target->intersect(state.geoContext, stepper.position(state), state.navDir * stepper.direction(state), false), true); - BOOST_CHECK_EQUAL(state.stepSize.value(), distance); + BOOST_CHECK_EQUAL(state.stepSize.value(), navDir * stepSize); } BOOST_AUTO_TEST_SUITE_END() diff --git a/Tests/UnitTests/Core/Propagator/CMakeLists.txt b/Tests/UnitTests/Core/Propagator/CMakeLists.txt index 1b06add2dd4..e3ba11aff87 100644 --- a/Tests/UnitTests/Core/Propagator/CMakeLists.txt +++ b/Tests/UnitTests/Core/Propagator/CMakeLists.txt @@ -13,6 +13,6 @@ add_unittest(MaterialCollection MaterialCollectionTests.cpp) add_unittest(MultiStepper MultiStepperTests.cpp) add_unittest(Navigator NavigatorTests.cpp) add_unittest(Propagator PropagatorTests.cpp) -add_unittest(Stepper StepperTests.cpp) +add_unittest(EigenStepper EigenStepperTests.cpp) add_unittest(StraightLineStepper StraightLineStepperTests.cpp) add_unittest(VolumeMaterialInteraction VolumeMaterialInteractionTests.cpp) diff --git a/Tests/UnitTests/Core/Propagator/ConstrainedStepTests.cpp b/Tests/UnitTests/Core/Propagator/ConstrainedStepTests.cpp index d3f8586306c..922f31747da 100644 --- a/Tests/UnitTests/Core/Propagator/ConstrainedStepTests.cpp +++ b/Tests/UnitTests/Core/Propagator/ConstrainedStepTests.cpp @@ -64,46 +64,6 @@ BOOST_AUTO_TEST_CASE(ConstrainedStepTest) { BOOST_CHECK_EQUAL(stepSize_p.value(ConstrainedStep::accuracy), std::numeric_limits::max()); BOOST_CHECK_EQUAL(stepSize_p.value(), 0.05); - - // backward stepping test - ConstrainedStep stepSize_n(-0.25); - - // All of the types should be 0.25 now - BOOST_CHECK_EQUAL(stepSize_n.value(ConstrainedStep::accuracy), - -std::numeric_limits::max()); - BOOST_CHECK_EQUAL(stepSize_n.value(ConstrainedStep::actor), - -std::numeric_limits::max()); - BOOST_CHECK_EQUAL(stepSize_n.value(ConstrainedStep::aborter), - -std::numeric_limits::max()); - BOOST_CHECK_EQUAL(stepSize_n.value(ConstrainedStep::user), -0.25); - - // Check the cast operation to double - BOOST_CHECK_EQUAL(stepSize_n.value(), -0.25); - - // now we update the accuracy - stepSize_n.update(-0.1, ConstrainedStep::accuracy); - BOOST_CHECK_EQUAL(stepSize_n.value(), -0.1); - - // now we update the actor to smaller - stepSize_n.update(-0.05, ConstrainedStep::actor); - BOOST_CHECK_EQUAL(stepSize_n.value(), -0.05); - // we increase the actor and accuracy is smaller again, without reset - stepSize_n.update(-0.15, ConstrainedStep::actor, false); - BOOST_CHECK_EQUAL(stepSize_n.value(), -0.05); - // we increase the actor and accuracy is smaller again, with reset - stepSize_n.update(-0.15, ConstrainedStep::actor, true); - BOOST_CHECK_EQUAL(stepSize_n.value(), -0.1); - - // now set two and update them - stepSize_n.update(-0.05, ConstrainedStep::user); - stepSize_n.update(-0.03, ConstrainedStep::accuracy); - BOOST_CHECK_EQUAL(stepSize_n.value(), -0.03); - - // now we release the accuracy - to the highest available value - stepSize_n.release(ConstrainedStep::accuracy); - BOOST_CHECK_EQUAL(stepSize_n.value(ConstrainedStep::accuracy), - -std::numeric_limits::max()); - BOOST_CHECK_EQUAL(stepSize_n.value(), -0.05); } } // namespace Test diff --git a/Tests/UnitTests/Core/Propagator/StepperTests.cpp b/Tests/UnitTests/Core/Propagator/EigenStepperTests.cpp similarity index 99% rename from Tests/UnitTests/Core/Propagator/StepperTests.cpp rename to Tests/UnitTests/Core/Propagator/EigenStepperTests.cpp index 7de4d4e674c..e753b82e215 100644 --- a/Tests/UnitTests/Core/Propagator/StepperTests.cpp +++ b/Tests/UnitTests/Core/Propagator/EigenStepperTests.cpp @@ -207,7 +207,7 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_state_test) { BOOST_CHECK_EQUAL(esState.cov, Covariance::Zero()); BOOST_CHECK_EQUAL(esState.navDir, navDir); BOOST_CHECK_EQUAL(esState.pathAccumulated, 0.); - BOOST_CHECK_EQUAL(esState.stepSize.value(), navDir * stepSize); + BOOST_CHECK_EQUAL(esState.stepSize.value(), stepSize); BOOST_CHECK_EQUAL(esState.previousStepSize, 0.); // Test without charge and covariance matrix @@ -264,12 +264,12 @@ BOOST_AUTO_TEST_CASE(eigen_stepper_test) { // Step size modifies const std::string originalStepSize = esState.stepSize.toString(); - es.setStepSize(esState, 1337.); - BOOST_CHECK_EQUAL(esState.previousStepSize, navDir * stepSize); - BOOST_CHECK_EQUAL(esState.stepSize.value(), 1337.); + es.setStepSize(esState, -1337.); + BOOST_CHECK_EQUAL(esState.previousStepSize, stepSize); + BOOST_CHECK_EQUAL(esState.stepSize.value(), -1337.); es.releaseStepSize(esState); - BOOST_CHECK_EQUAL(esState.stepSize.value(), -123.); + BOOST_CHECK_EQUAL(esState.stepSize.value(), stepSize); BOOST_CHECK_EQUAL(es.outputStepSize(esState), originalStepSize); // Test the curvilinear state construction diff --git a/Tests/UnitTests/Core/Propagator/MaterialCollectionTests.cpp b/Tests/UnitTests/Core/Propagator/MaterialCollectionTests.cpp index ff73dba390d..69b2f72bb17 100644 --- a/Tests/UnitTests/Core/Propagator/MaterialCollectionTests.cpp +++ b/Tests/UnitTests/Core/Propagator/MaterialCollectionTests.cpp @@ -169,7 +169,7 @@ void runTest(const propagator_t& prop, double pT, double phi, double theta, // backward material test Options bwdOptions(tgContext, mfContext); - bwdOptions.maxStepSize = -25_cm; + bwdOptions.maxStepSize = 25_cm; bwdOptions.pathLimit = -25_cm; bwdOptions.direction = Direction::Backward; @@ -305,7 +305,7 @@ void runTest(const propagator_t& prop, double pT, double phi, double theta, // now go from surface to surface and check Options bwdStepOptions(tgContext, mfContext); - bwdStepOptions.maxStepSize = -25_cm; + bwdStepOptions.maxStepSize = 25_cm; bwdStepOptions.pathLimit = -25_cm; bwdStepOptions.direction = Direction::Backward; diff --git a/Tests/UnitTests/Core/Propagator/PropagatorTests.cpp b/Tests/UnitTests/Core/Propagator/PropagatorTests.cpp index f7af1af9582..59b3538b449 100644 --- a/Tests/UnitTests/Core/Propagator/PropagatorTests.cpp +++ b/Tests/UnitTests/Core/Propagator/PropagatorTests.cpp @@ -114,7 +114,8 @@ struct SurfaceObserver { stepper.direction(state.stepping), true) .intersection.pathLength; // Adjust the step size so that we cannot cross the target surface - state.stepping.stepSize.update(distance, ConstrainedStep::actor); + state.stepping.stepSize.update(distance * state.stepping.navDir, + ConstrainedStep::actor); // return true if you fall below tolerance if (std::abs(distance) <= tolerance) { ++result.surfaces_passed; diff --git a/Tests/UnitTests/Core/Propagator/StraightLineStepperTests.cpp b/Tests/UnitTests/Core/Propagator/StraightLineStepperTests.cpp index e40ac172240..ce44cab69e3 100644 --- a/Tests/UnitTests/Core/Propagator/StraightLineStepperTests.cpp +++ b/Tests/UnitTests/Core/Propagator/StraightLineStepperTests.cpp @@ -99,7 +99,7 @@ BOOST_AUTO_TEST_CASE(straight_line_stepper_state_test) { CHECK_CLOSE_OR_SMALL(sls.time(slsState), time, eps, eps); BOOST_CHECK_EQUAL(slsState.navDir, navDir); BOOST_CHECK_EQUAL(slsState.pathAccumulated, 0.); - BOOST_CHECK_EQUAL(slsState.stepSize.value(), navDir * stepSize); + BOOST_CHECK_EQUAL(slsState.stepSize.value(), stepSize); BOOST_CHECK_EQUAL(slsState.previousStepSize, 0.); BOOST_CHECK_EQUAL(slsState.tolerance, tolerance); @@ -158,12 +158,12 @@ BOOST_AUTO_TEST_CASE(straight_line_stepper_test) { // Step size modifies const std::string originalStepSize = slsState.stepSize.toString(); - sls.setStepSize(slsState, 1337.); - BOOST_CHECK_EQUAL(slsState.previousStepSize, navDir * stepSize); - BOOST_CHECK_EQUAL(slsState.stepSize.value(), 1337.); + sls.setStepSize(slsState, -1337.); + BOOST_CHECK_EQUAL(slsState.previousStepSize, stepSize); + BOOST_CHECK_EQUAL(slsState.stepSize.value(), -1337.); sls.releaseStepSize(slsState); - BOOST_CHECK_EQUAL(slsState.stepSize.value(), -123.); + BOOST_CHECK_EQUAL(slsState.stepSize.value(), stepSize); BOOST_CHECK_EQUAL(sls.outputStepSize(slsState), originalStepSize); // Test the curvilinear state construction @@ -205,8 +205,8 @@ BOOST_AUTO_TEST_CASE(straight_line_stepper_test) { ps.stepping.covTransport = false; double h = sls.step(ps, mockNavigator).value(); - BOOST_CHECK_EQUAL(ps.stepping.stepSize.value(), navDir * stepSize); - BOOST_CHECK_EQUAL(ps.stepping.stepSize.value(), h); + BOOST_CHECK_EQUAL(ps.stepping.stepSize.value(), stepSize); + BOOST_CHECK_EQUAL(ps.stepping.stepSize.value(), h * navDir); CHECK_CLOSE_COVARIANCE(ps.stepping.cov, cov, 1e-6); BOOST_CHECK_GT(sls.position(ps.stepping).norm(), newPos.norm()); CHECK_CLOSE_ABS(sls.direction(ps.stepping), newMom.normalized(), 1e-6); @@ -218,7 +218,7 @@ BOOST_AUTO_TEST_CASE(straight_line_stepper_test) { ps.stepping.covTransport = true; double h2 = sls.step(ps, mockNavigator).value(); - BOOST_CHECK_EQUAL(ps.stepping.stepSize.value(), navDir * stepSize); + BOOST_CHECK_EQUAL(ps.stepping.stepSize.value(), stepSize); BOOST_CHECK_EQUAL(h2, h); CHECK_CLOSE_COVARIANCE(ps.stepping.cov, cov, 1e-6); BOOST_CHECK_GT(sls.position(ps.stepping).norm(), newPos.norm());