Skip to content

Commit

Permalink
refactor!: Remove alias IntersectionStatus::missed (#3899)
Browse files Browse the repository at this point in the history
I believe such an alias is increasing confusing. I propose to remove `IntersectionStatus::missed` and replace it by `IntersectionStatus::unreachable`
  • Loading branch information
andiwand authored Nov 25, 2024
1 parent c5391be commit b0f6bde
Show file tree
Hide file tree
Showing 11 changed files with 22 additions and 25 deletions.
11 changes: 2 additions & 9 deletions Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -36,7 +32,6 @@
#include <cstddef>
#include <functional>
#include <limits>
#include <numeric>
#include <sstream>
#include <vector>

Expand Down Expand Up @@ -398,7 +393,7 @@ class MultiEigenStepperLoop : public EigenStepper<extension_t> {
void removeMissedComponents(State& state) const {
auto new_end = std::remove_if(
state.components.begin(), state.components.end(), [](const auto& cmp) {
return cmp.status == IntersectionStatus::missed;
return cmp.status == IntersectionStatus::unreachable;
});

state.components.erase(new_end, state.components.end());
Expand Down Expand Up @@ -581,10 +576,8 @@ class MultiEigenStepperLoop : public EigenStepper<extension_t> {
} else if (counts[static_cast<std::size_t>(Status::onSurface)] > 0) {
state.stepCounterAfterFirstComponentOnSurface.reset();
return Status::onSurface;
} else if (counts[static_cast<std::size_t>(Status::unreachable)] > 0) {
return Status::unreachable;
} else {
return Status::missed;
return Status::unreachable;
}
}

Expand Down
3 changes: 2 additions & 1 deletion Core/include/Acts/Propagator/MultiEigenStepperLoop.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -112,7 +113,7 @@ Result<double> MultiEigenStepperLoop<E, R>::step(
m_stepLimitAfterFirstComponentOnSurface) {
for (auto& cmp : components) {
if (cmp.status != Status::onSurface) {
cmp.status = Status::missed;
cmp.status = Status::unreachable;
}
}

Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/TrackFitting/detail/GsfActor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,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() = IntersectionStatus::missed;
cmp.status() = IntersectionStatus::unreachable;
continue;
}

Expand Down
3 changes: 1 addition & 2 deletions Core/include/Acts/Utilities/Intersection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ namespace Acts {

/// Status enum
enum class IntersectionStatus : int {
missed = 0,
unreachable = 0,
reachable = 1,
onSurface = 2
Expand Down Expand Up @@ -58,7 +57,7 @@ class Intersection {

/// Returns whether the intersection was successful or not
constexpr bool isValid() const {
return m_status != IntersectionStatus::missed;
return m_status != IntersectionStatus::unreachable;
}

/// Returns the position of the interseciton
Expand Down
4 changes: 2 additions & 2 deletions Core/src/Surfaces/ConeSurface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ Acts::SurfaceMultiIntersection Acts::ConeSurface::intersect(

if (!boundaryTolerance.isInfinite() &&
!isOnSurface(gctx, solution1, direction, boundaryTolerance)) {
status1 = IntersectionStatus::missed;
status1 = IntersectionStatus::unreachable;
}

// Check the validity of the second solution
Expand All @@ -311,7 +311,7 @@ Acts::SurfaceMultiIntersection Acts::ConeSurface::intersect(
: IntersectionStatus::reachable;
if (!boundaryTolerance.isInfinite() &&
!isOnSurface(gctx, solution2, direction, boundaryTolerance)) {
status2 = IntersectionStatus::missed;
status2 = IntersectionStatus::unreachable;
}

const auto& tf = transform(gctx);
Expand Down
5 changes: 3 additions & 2 deletions Core/src/Surfaces/CylinderSurface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,12 @@ Acts::SurfaceMultiIntersection Acts::CylinderSurface::intersect(
double cZ = vecLocal.dot(tMatrix.block<3, 1>(0, 2));
double modifiedTolerance = tolerance + absoluteBound->tolerance1;
double hZ = cBounds.get(CylinderBounds::eHalfLengthZ) + modifiedTolerance;
return std::abs(cZ) < std::abs(hZ) ? status : IntersectionStatus::missed;
return std::abs(cZ) < std::abs(hZ) ? status
: IntersectionStatus::unreachable;
}
return isOnSurface(gctx, solution, direction, boundaryTolerance)
? status
: IntersectionStatus::missed;
: IntersectionStatus::unreachable;
};
// Check first solution for boundary compatibility
status1 = boundaryCheck(solution1, status1);
Expand Down
4 changes: 2 additions & 2 deletions Core/src/Surfaces/DiscSurface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,11 @@ Acts::SurfaceMultiIntersection Acts::DiscSurface::intersect(
double modifiedTolerance = tolerance + absoluteBound->tolerance0;
if (!m_bounds->insideRadialBounds(VectorHelpers::perp(lcartesian),
modifiedTolerance)) {
status = IntersectionStatus::missed;
status = IntersectionStatus::unreachable;
}
} else if (!insideBounds(localCartesianToPolar(lcartesian),
boundaryTolerance)) {
status = IntersectionStatus::missed;
status = IntersectionStatus::unreachable;
}
}
return {{Intersection3D(intersection.position(), intersection.pathLength(),
Expand Down
2 changes: 1 addition & 1 deletion Core/src/Surfaces/LineSurface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 = IntersectionStatus::missed;
status = IntersectionStatus::unreachable;
}
}

Expand Down
2 changes: 1 addition & 1 deletion Core/src/Surfaces/PlaneSurface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 = IntersectionStatus::missed;
status = IntersectionStatus::unreachable;
}
}
return {{Intersection3D(intersection.position(), intersection.pathLength(),
Expand Down
9 changes: 6 additions & 3 deletions Tests/UnitTests/Core/Surfaces/SurfaceIntersectionTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ BOOST_AUTO_TEST_CASE(CylinderIntersectionTests) {
// There MUST be a second solution
BOOST_CHECK(!eIntersection[1].isValid());
// The other intersection MUST NOT be reachable
BOOST_CHECK_EQUAL(eIntersection[1].status(), IntersectionStatus::missed);
BOOST_CHECK_EQUAL(eIntersection[1].status(),
IntersectionStatus::unreachable);
// And be the positive one
BOOST_CHECK_GT(eIntersection[1].pathLength(), 0.);
};
Expand Down Expand Up @@ -279,7 +280,8 @@ BOOST_AUTO_TEST_CASE(PlanarIntersectionTest) {
// The intersection MUST NOT be valid
BOOST_CHECK(!mIntersection[0].isValid());
// The intersection MUST be reachable
BOOST_CHECK_EQUAL(mIntersection[0].status(), IntersectionStatus::missed);
BOOST_CHECK_EQUAL(mIntersection[0].status(),
IntersectionStatus::unreachable);
// The path length MUST be negative
BOOST_CHECK_GT(mIntersection[0].pathLength(), 0.);
// The intersection MUST be unique
Expand Down Expand Up @@ -386,7 +388,8 @@ BOOST_AUTO_TEST_CASE(LineIntersectionTest) {
// The intersection MUST NOT be valid
BOOST_CHECK(!mIntersection[0].isValid());
// The intersection MUST be reachable
BOOST_CHECK_EQUAL(mIntersection[0].status(), IntersectionStatus::missed);
BOOST_CHECK_EQUAL(mIntersection[0].status(),
IntersectionStatus::unreachable);
// The path length MUST be negative
BOOST_CHECK_LT(mIntersection[0].pathLength(), 0.);
// The intersection MUST be unique
Expand Down
2 changes: 1 addition & 1 deletion Tests/UnitTests/Core/Utilities/IntersectionTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ BOOST_AUTO_TEST_CASE(ObjectIntersectionTest) {

BOOST_AUTO_TEST_CASE(IntersectionStatusPrinting) {
std::array<IntersectionStatus, 4> status_values = {
{IntersectionStatus::missed, IntersectionStatus::unreachable,
{IntersectionStatus::unreachable, IntersectionStatus::unreachable,
IntersectionStatus::reachable, IntersectionStatus::onSurface}};
std::array<std::string, 4> expected_messages = {
{"missed/unreachable", "missed/unreachable", "reachable", "onSurface"}};
Expand Down

0 comments on commit b0f6bde

Please sign in to comment.