From 78b39eb82ab53c362b7bf74f1fe133914e18cd8d Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sat, 23 Nov 2024 17:52:02 +0100 Subject: [PATCH 1/2] refactor!: Remove alias `IntersectionStatus::missed` --- Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp | 4 ++-- Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp | 2 +- Core/include/Acts/TrackFitting/detail/GsfActor.hpp | 2 +- Core/include/Acts/Utilities/Intersection.hpp | 3 +-- Core/src/Surfaces/ConeSurface.cpp | 4 ++-- Core/src/Surfaces/CylinderSurface.cpp | 4 ++-- Core/src/Surfaces/DiscSurface.cpp | 4 ++-- Core/src/Surfaces/LineSurface.cpp | 2 +- Core/src/Surfaces/PlaneSurface.cpp | 2 +- Tests/UnitTests/Core/Surfaces/SurfaceIntersectionTests.cpp | 6 +++--- Tests/UnitTests/Core/Utilities/IntersectionTests.cpp | 2 +- 11 files changed, 17 insertions(+), 18 deletions(-) diff --git a/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp b/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp index e21bdb2365e..9e9a0a9fe2c 100644 --- a/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp +++ b/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp @@ -398,7 +398,7 @@ class MultiEigenStepperLoop : public EigenStepper { void removeMissedComponents(State& state) const { auto new_end = std::remove_if( state.components.begin(), state.components.end(), [](const auto& cmp) { - return cmp.status == Intersection3D::Status::missed; + return cmp.status == Intersection3D::Status::unreachable; }); state.components.erase(new_end, state.components.end()); @@ -584,7 +584,7 @@ class MultiEigenStepperLoop : public EigenStepper { } else if (counts[static_cast(Status::unreachable)] > 0) { return Status::unreachable; } else { - return Status::missed; + return Status::unreachable; } } diff --git a/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp b/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp index fab3e3f0779..20f4f8933cc 100644 --- a/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp +++ b/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp @@ -112,7 +112,7 @@ Result MultiEigenStepperLoop::step( m_stepLimitAfterFirstComponentOnSurface) { for (auto& cmp : components) { if (cmp.status != Status::onSurface) { - cmp.status = Status::missed; + cmp.status = Status::unreachable; } } diff --git a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp index 92c5529d319..43801c320ef 100644 --- a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp +++ b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp @@ -484,7 +484,7 @@ struct GsfActor { // we set ignored components to missed, so we can remove them after // the loop if (tmpStates.weights.at(idx) < m_cfg.weightCutoff) { - cmp.status() = Intersection3D::Status::missed; + cmp.status() = Intersection3D::Status::unreachable; continue; } diff --git a/Core/include/Acts/Utilities/Intersection.hpp b/Core/include/Acts/Utilities/Intersection.hpp index 6276538ed5e..287e37ee5b9 100644 --- a/Core/include/Acts/Utilities/Intersection.hpp +++ b/Core/include/Acts/Utilities/Intersection.hpp @@ -24,7 +24,6 @@ namespace Acts { /// Status enum enum class IntersectionStatus : int { - missed = 0, unreachable = 0, reachable = 1, onSurface = 2 @@ -60,7 +59,7 @@ class Intersection { : m_position(position), m_pathLength(pathLength), m_status(status) {} /// Returns whether the intersection was successful or not - constexpr bool isValid() const { return m_status != Status::missed; } + constexpr bool isValid() const { return m_status != Status::unreachable; } /// Returns the position of the interseciton constexpr const Position& position() const { return m_position; } diff --git a/Core/src/Surfaces/ConeSurface.cpp b/Core/src/Surfaces/ConeSurface.cpp index b6d04e87e06..688e80b1b6c 100644 --- a/Core/src/Surfaces/ConeSurface.cpp +++ b/Core/src/Surfaces/ConeSurface.cpp @@ -301,7 +301,7 @@ Acts::SurfaceMultiIntersection Acts::ConeSurface::intersect( if (!boundaryTolerance.isInfinite() && !isOnSurface(gctx, solution1, direction, boundaryTolerance)) { - status1 = Intersection3D::Status::missed; + status1 = Intersection3D::Status::unreachable; } // Check the validity of the second solution @@ -311,7 +311,7 @@ Acts::SurfaceMultiIntersection Acts::ConeSurface::intersect( : Intersection3D::Status::reachable; if (!boundaryTolerance.isInfinite() && !isOnSurface(gctx, solution2, direction, boundaryTolerance)) { - status2 = Intersection3D::Status::missed; + status2 = Intersection3D::Status::unreachable; } const auto& tf = transform(gctx); diff --git a/Core/src/Surfaces/CylinderSurface.cpp b/Core/src/Surfaces/CylinderSurface.cpp index 4b3cd2db7be..132ed8260a5 100644 --- a/Core/src/Surfaces/CylinderSurface.cpp +++ b/Core/src/Surfaces/CylinderSurface.cpp @@ -267,11 +267,11 @@ Acts::SurfaceMultiIntersection Acts::CylinderSurface::intersect( double modifiedTolerance = tolerance + absoluteBound->tolerance1; double hZ = cBounds.get(CylinderBounds::eHalfLengthZ) + modifiedTolerance; return std::abs(cZ) < std::abs(hZ) ? status - : Intersection3D::Status::missed; + : Intersection3D::Status::unreachable; } return isOnSurface(gctx, solution, direction, boundaryTolerance) ? status - : Intersection3D::Status::missed; + : Intersection3D::Status::unreachable; }; // Check first solution for boundary compatibility status1 = boundaryCheck(solution1, status1); diff --git a/Core/src/Surfaces/DiscSurface.cpp b/Core/src/Surfaces/DiscSurface.cpp index da1aa2b54bc..dbaaeb5edb9 100644 --- a/Core/src/Surfaces/DiscSurface.cpp +++ b/Core/src/Surfaces/DiscSurface.cpp @@ -297,11 +297,11 @@ Acts::SurfaceMultiIntersection Acts::DiscSurface::intersect( double modifiedTolerance = tolerance + absoluteBound->tolerance0; if (!m_bounds->insideRadialBounds(VectorHelpers::perp(lcartesian), modifiedTolerance)) { - status = Intersection3D::Status::missed; + status = Intersection3D::Status::unreachable; } } else if (!insideBounds(localCartesianToPolar(lcartesian), boundaryTolerance)) { - status = Intersection3D::Status::missed; + status = Intersection3D::Status::unreachable; } } return {{Intersection3D(intersection.position(), intersection.pathLength(), diff --git a/Core/src/Surfaces/LineSurface.cpp b/Core/src/Surfaces/LineSurface.cpp index f67fe038aa9..29d9190e58d 100644 --- a/Core/src/Surfaces/LineSurface.cpp +++ b/Core/src/Surfaces/LineSurface.cpp @@ -185,7 +185,7 @@ Acts::SurfaceMultiIntersection Acts::LineSurface::intersect( double cZ = vecLocal.dot(eb); double cR = (vecLocal - cZ * eb).norm(); if (!m_bounds->inside({cR, cZ}, boundaryTolerance)) { - status = Intersection3D::Status::missed; + status = Intersection3D::Status::unreachable; } } diff --git a/Core/src/Surfaces/PlaneSurface.cpp b/Core/src/Surfaces/PlaneSurface.cpp index 658920e6686..67c42cacd55 100644 --- a/Core/src/Surfaces/PlaneSurface.cpp +++ b/Core/src/Surfaces/PlaneSurface.cpp @@ -175,7 +175,7 @@ Acts::SurfaceMultiIntersection Acts::PlaneSurface::intersect( const Vector3 vecLocal(intersection.position() - tMatrix.block<3, 1>(0, 3)); if (!insideBounds(tMatrix.block<3, 2>(0, 0).transpose() * vecLocal, boundaryTolerance)) { - status = Intersection3D::Status::missed; + status = Intersection3D::Status::unreachable; } } return {{Intersection3D(intersection.position(), intersection.pathLength(), diff --git a/Tests/UnitTests/Core/Surfaces/SurfaceIntersectionTests.cpp b/Tests/UnitTests/Core/Surfaces/SurfaceIntersectionTests.cpp index e6294fc27ca..1d741c2f799 100644 --- a/Tests/UnitTests/Core/Surfaces/SurfaceIntersectionTests.cpp +++ b/Tests/UnitTests/Core/Surfaces/SurfaceIntersectionTests.cpp @@ -155,7 +155,7 @@ BOOST_AUTO_TEST_CASE(CylinderIntersectionTests) { BOOST_CHECK(!eIntersection[1].isValid()); // The other intersection MUST NOT be reachable BOOST_CHECK_EQUAL(eIntersection[1].status(), - Intersection3D::Status::missed); + Intersection3D::Status::unreachable); // And be the positive one BOOST_CHECK_GT(eIntersection[1].pathLength(), 0.); }; @@ -295,7 +295,7 @@ BOOST_AUTO_TEST_CASE(PlanarIntersectionTest) { BOOST_CHECK(!mIntersection[0].isValid()); // The intersection MUST be reachable BOOST_CHECK_EQUAL(mIntersection[0].status(), - Intersection3D::Status::missed); + Intersection3D::Status::unreachable); // The path length MUST be negative BOOST_CHECK_GT(mIntersection[0].pathLength(), 0.); // The intersection MUST be unique @@ -407,7 +407,7 @@ BOOST_AUTO_TEST_CASE(LineIntersectionTest) { BOOST_CHECK(!mIntersection[0].isValid()); // The intersection MUST be reachable BOOST_CHECK_EQUAL(mIntersection[0].status(), - Intersection3D::Status::missed); + Intersection3D::Status::unreachable); // The path length MUST be negative BOOST_CHECK_LT(mIntersection[0].pathLength(), 0.); // The intersection MUST be unique diff --git a/Tests/UnitTests/Core/Utilities/IntersectionTests.cpp b/Tests/UnitTests/Core/Utilities/IntersectionTests.cpp index 41581b08b8d..0b2ae3f2ffe 100644 --- a/Tests/UnitTests/Core/Utilities/IntersectionTests.cpp +++ b/Tests/UnitTests/Core/Utilities/IntersectionTests.cpp @@ -183,7 +183,7 @@ BOOST_AUTO_TEST_CASE(ObjectIntersectionTest) { BOOST_AUTO_TEST_CASE(IntersectionStatusPrinting) { std::array status_values = { - {IntersectionStatus::missed, IntersectionStatus::unreachable, + {IntersectionStatus::unreachable, IntersectionStatus::unreachable, IntersectionStatus::reachable, IntersectionStatus::onSurface}}; std::array expected_messages = { {"missed/unreachable", "missed/unreachable", "reachable", "onSurface"}}; From c3407f9e800508d6c7fa2c19817a609e983b609c Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sat, 23 Nov 2024 21:01:08 +0100 Subject: [PATCH 2/2] pr feedback --- Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp | 7 ------- Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp | 1 + 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp b/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp index 9e9a0a9fe2c..143d8946bff 100644 --- a/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp +++ b/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp @@ -14,16 +14,12 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/Definitions/Direction.hpp" #include "Acts/Definitions/TrackParametrization.hpp" -#include "Acts/Definitions/Units.hpp" #include "Acts/EventData/MultiComponentTrackParameters.hpp" #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/MultiStepperError.hpp" -#include "Acts/Propagator/Propagator.hpp" #include "Acts/Propagator/StepperOptions.hpp" #include "Acts/Propagator/StepperStatistics.hpp" #include "Acts/Propagator/detail/LoopStepperUtils.hpp" @@ -36,7 +32,6 @@ #include #include #include -#include #include #include @@ -581,8 +576,6 @@ class MultiEigenStepperLoop : public EigenStepper { } else if (counts[static_cast(Status::onSurface)] > 0) { state.stepCounterAfterFirstComponentOnSurface.reset(); return Status::onSurface; - } else if (counts[static_cast(Status::unreachable)] > 0) { - return Status::unreachable; } else { return Status::unreachable; } diff --git a/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp b/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp index 20f4f8933cc..3cd6577e004 100644 --- a/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp +++ b/Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp @@ -7,6 +7,7 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. #include "Acts/Propagator/MultiEigenStepperLoop.hpp" +#include "Acts/Propagator/MultiStepperError.hpp" #include "Acts/Utilities/Logger.hpp" namespace Acts {