Skip to content

Commit

Permalink
refactor!: Use enum class for NavigationDirection (#1206)
Browse files Browse the repository at this point in the history
This change is motivated by arithmetic between non-class enums and other types that are not the same enum is deprecated in C++20. I overload the operators we use these enums in to support the same use cases.

I think this is breaking since it's part of the public propagator interface.

BREAKING CHANGE: The `NavigationDirection` enum changes to `enum class`. The values are renamed `NavigationDiretcion::forward` -> `NavigationDirection::Forward` and `NavigationDirection::backward` -> `NavigationDirection::Backward`.
  • Loading branch information
paulgessinger committed Apr 8, 2022
1 parent 65d1e09 commit 394cfdb
Show file tree
Hide file tree
Showing 45 changed files with 282 additions and 144 deletions.
63 changes: 62 additions & 1 deletion Core/include/Acts/Definitions/Common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,68 @@ static constexpr ActsScalar s_curvilinearProjTolerance = 0.999995;
/// @enum NavigationDirection
/// The navigation direciton is always with
/// respect to a given momentum or direction
enum NavigationDirection : int { backward = -1, forward = 1 };
enum class NavigationDirection : int { Backward = -1, Forward = 1 };

std::ostream& operator<<(std::ostream& os, NavigationDirection navDir);

// NavigationDirection * T

inline constexpr auto operator*(NavigationDirection dir, int value) {
return static_cast<std::underlying_type_t<NavigationDirection>>(dir) * value;
}

inline constexpr auto operator*(NavigationDirection dir, float value) {
return static_cast<std::underlying_type_t<NavigationDirection>>(dir) * value;
}

inline constexpr auto operator*(NavigationDirection dir, double value) {
return static_cast<std::underlying_type_t<NavigationDirection>>(dir) * value;
}

inline Acts::Vector3 operator*(NavigationDirection dir, Acts::Vector3 value) {
return static_cast<std::underlying_type_t<NavigationDirection>>(dir) * value;
}

// T * NavigationDirection

inline constexpr auto operator*(int value, NavigationDirection dir) {
return value * static_cast<std::underlying_type_t<NavigationDirection>>(dir);
}

inline constexpr auto operator*(float value, NavigationDirection dir) {
return value * static_cast<std::underlying_type_t<NavigationDirection>>(dir);
}

inline constexpr auto operator*(double value, NavigationDirection dir) {
return value * static_cast<std::underlying_type_t<NavigationDirection>>(dir);
}

inline Acts::Vector3 operator*(Acts::Vector3 value, NavigationDirection dir) {
return value * static_cast<std::underlying_type_t<NavigationDirection>>(dir);
}

// T *= NavigationDirection

inline constexpr auto operator*=(int& value, NavigationDirection dir) {
value *= static_cast<std::underlying_type_t<NavigationDirection>>(dir);
return value;
}

inline constexpr auto operator*=(float& value, NavigationDirection dir) {
value *= static_cast<std::underlying_type_t<NavigationDirection>>(dir);
return value;
}

inline constexpr auto operator*=(double& value, NavigationDirection dir) {
value *= static_cast<std::underlying_type_t<NavigationDirection>>(dir);
return value;
}

inline Acts::Vector3& operator*=(Acts::Vector3& value,
NavigationDirection dir) {
value *= static_cast<std::underlying_type_t<NavigationDirection>>(dir);
return value;
}

/// This is a steering enum to tell which material update stage:
/// - PreUpdate : update on approach of a surface
Expand Down
4 changes: 2 additions & 2 deletions Core/include/Acts/Geometry/BoundarySurfaceT.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ inline const Surface& BoundarySurfaceT<volume_t>::surfaceRepresentation()
template <class volume_t>
void BoundarySurfaceT<volume_t>::attachVolume(const volume_t* volume,
NavigationDirection navDir) {
if (navDir == backward) {
if (navDir == NavigationDirection::Backward) {
m_oppositeVolume = volume;
} else {
m_alongVolume = volume;
Expand All @@ -170,7 +170,7 @@ template <class volume_t>
void BoundarySurfaceT<volume_t>::attachVolumeArray(
const std::shared_ptr<const VolumeArray> volumes,
NavigationDirection navDir) {
if (navDir == backward) {
if (navDir == NavigationDirection::Backward) {
m_oppositeVolumeArray = volumes;
} else {
m_alongVolumeArray = volumes;
Expand Down
4 changes: 2 additions & 2 deletions Core/include/Acts/Material/ISurfaceMaterial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@ inline double ISurfaceMaterial::factor(NavigationDirection pDir,
if (mStage == Acts::MaterialUpdateStage::FullUpdate) {
return 1.;
} else if (mStage == Acts::MaterialUpdateStage::PreUpdate) {
return pDir == NavigationDirection::backward ? m_splitFactor
return pDir == NavigationDirection::Backward ? m_splitFactor
: 1 - m_splitFactor;
} else /*if (mStage == Acts::MaterialUpdateStage::PostUpdate)*/ {
return pDir == NavigationDirection::forward ? m_splitFactor
return pDir == NavigationDirection::Forward ? m_splitFactor
: 1 - m_splitFactor;
}
}
Expand Down
7 changes: 4 additions & 3 deletions Core/include/Acts/Propagator/AtlasStepper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class AtlasStepper {
template <typename Parameters>
State(const GeometryContext& gctx,
MagneticFieldProvider::Cache fieldCacheIn, const Parameters& pars,
NavigationDirection ndir = forward,
NavigationDirection ndir = NavigationDirection::Forward,
double ssize = std::numeric_limits<double>::max(),
double stolerance = s_onSurfaceTolerance)
: navDir(ndir),
Expand Down Expand Up @@ -305,7 +305,7 @@ class AtlasStepper {
State makeState(std::reference_wrapper<const GeometryContext> gctx,
std::reference_wrapper<const MagneticFieldContext> mctx,
const SingleBoundTrackParameters<charge_t>& par,
NavigationDirection ndir = forward,
NavigationDirection ndir = NavigationDirection::Forward,
double ssize = std::numeric_limits<double>::max(),
double stolerance = s_onSurfaceTolerance) const {
return State{gctx, m_bField->makeCache(mctx), par, ndir, ssize, stolerance};
Expand All @@ -321,7 +321,8 @@ class AtlasStepper {
/// @param [in] stepSize Step size
void resetState(
State& state, const BoundVector& boundParams, const BoundSymMatrix& cov,
const Surface& surface, const NavigationDirection navDir = forward,
const Surface& surface,
const NavigationDirection navDir = NavigationDirection::Forward,
const double stepSize = std::numeric_limits<double>::max()) const {
// Update the stepping state
update(state,
Expand Down
15 changes: 9 additions & 6 deletions Core/include/Acts/Propagator/ConstrainedStep.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct ConstrainedStep {
std::numeric_limits<Scalar>::max(), std::numeric_limits<Scalar>::max()}};

/// The Navigation direction
NavigationDirection direction = forward;
NavigationDirection direction = NavigationDirection::Forward;

/// Update the step size of a certain type
///
Expand All @@ -61,15 +61,17 @@ struct ConstrainedStep {
///
/// @param type is the constraint type to be released
void release(Type type) {
Scalar mvalue = (direction == forward)
Scalar mvalue = (direction == NavigationDirection::Forward)
? (*std::max_element(values.begin(), values.end()))
: (*std::min_element(values.begin(), values.end()));
values[type] = mvalue;
}

/// constructor from double
/// @param value is the user given initial value
ConstrainedStep(Scalar value) : direction(value > 0. ? forward : backward) {
ConstrainedStep(Scalar value)
: direction(value > 0. ? NavigationDirection::Forward
: NavigationDirection::Backward) {
values[accuracy] *= direction;
values[actor] *= direction;
values[aborter] *= direction;
Expand All @@ -85,14 +87,15 @@ struct ConstrainedStep {
/// set the accuracy value
values[accuracy] = value;
// set/update the direction
direction = value > 0. ? forward : backward;
direction = value > 0. ? NavigationDirection::Forward
: NavigationDirection::Backward;
return (*this);
}

/// Cast operator to double, returning the min/max value
/// depending on the direction
operator Scalar() const {
if (direction == forward) {
if (direction == NavigationDirection::Forward) {
return (*std::min_element(values.begin(), values.end()));
}
return (*std::max_element(values.begin(), values.end()));
Expand All @@ -118,7 +121,7 @@ struct ConstrainedStep {
/// Access to currently leading min type
///
Type currentType() const {
if (direction == forward) {
if (direction == NavigationDirection::Forward) {
return Type(std::min_element(values.begin(), values.end()) -
values.begin());
}
Expand Down
7 changes: 4 additions & 3 deletions Core/include/Acts/Propagator/EigenStepper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class EigenStepper {
explicit State(const GeometryContext& gctx,
MagneticFieldProvider::Cache fieldCacheIn,
const SingleBoundTrackParameters<charge_t>& par,
NavigationDirection ndir = forward,
NavigationDirection ndir = NavigationDirection::Forward,
double ssize = std::numeric_limits<double>::max(),
double stolerance = s_onSurfaceTolerance)
: q(par.charge()),
Expand Down Expand Up @@ -170,7 +170,7 @@ class EigenStepper {
State makeState(std::reference_wrapper<const GeometryContext> gctx,
std::reference_wrapper<const MagneticFieldContext> mctx,
const SingleBoundTrackParameters<charge_t>& par,
NavigationDirection ndir = forward,
NavigationDirection ndir = NavigationDirection::Forward,
double ssize = std::numeric_limits<double>::max(),
double stolerance = s_onSurfaceTolerance) const;

Expand All @@ -184,7 +184,8 @@ class EigenStepper {
/// @param [in] stepSize Step size
void resetState(
State& state, const BoundVector& boundParams, const BoundSymMatrix& cov,
const Surface& surface, const NavigationDirection navDir = forward,
const Surface& surface,
const NavigationDirection navDir = NavigationDirection::Forward,
const double stepSize = std::numeric_limits<double>::max()) const;

/// Get the field for the stepping, it checks first if the access is still
Expand Down
3 changes: 2 additions & 1 deletion Core/include/Acts/Propagator/MaterialInteractor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ struct MaterialInteractor {
}
// Change the noise updater depending on the navigation direction
NoiseUpdateMode mode =
(state.stepping.navDir == forward) ? addNoise : removeNoise;
(state.stepping.navDir == NavigationDirection::Forward) ? addNoise
: removeNoise;
// Apply the material interactions
d.updateState(state, stepper, mode);
// Record the result
Expand Down
7 changes: 4 additions & 3 deletions Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ class MultiEigenStepperLoop
const GeometryContext& gctx, const MagneticFieldContext& mctx,
const std::shared_ptr<const MagneticFieldProvider>& bfield,
const MultiComponentBoundTrackParameters<charge_t>& multipars,
NavigationDirection ndir = forward,
NavigationDirection ndir = NavigationDirection::Forward,
double ssize = std::numeric_limits<double>::max(),
double stolerance = s_onSurfaceTolerance)
: navDir(ndir), geoContext(gctx), magContext(mctx) {
Expand Down Expand Up @@ -290,7 +290,7 @@ class MultiEigenStepperLoop
State makeState(std::reference_wrapper<const GeometryContext> gctx,
std::reference_wrapper<const MagneticFieldContext> mctx,
const MultiComponentBoundTrackParameters<charge_t>& par,
NavigationDirection ndir = forward,
NavigationDirection ndir = NavigationDirection::Forward,
double ssize = std::numeric_limits<double>::max(),
double stolerance = s_onSurfaceTolerance) const {
return State(gctx, mctx, SingleStepper::m_bField, par, ndir, ssize,
Expand All @@ -307,7 +307,8 @@ class MultiEigenStepperLoop
/// @param [in] stepSize Step size
void resetState(
State& state, const BoundVector& boundParams, const BoundSymMatrix& cov,
const Surface& surface, const NavigationDirection navDir = forward,
const Surface& surface,
const NavigationDirection navDir = NavigationDirection::Forward,
const double stepSize = std::numeric_limits<double>::max()) const {
for (auto& component : state.components) {
SingleStepper::resetState(component.state, boundParams, cov, surface,
Expand Down
4 changes: 2 additions & 2 deletions Core/include/Acts/Propagator/Navigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace Acts {
template <typename object_t>
struct NavigationOptions {
/// The navigation direction
NavigationDirection navDir = forward;
NavigationDirection navDir = NavigationDirection::Forward;

/// The boundary check directive
BoundaryCheck boundaryCheck = true;
Expand Down Expand Up @@ -765,7 +765,7 @@ class Navigator {
// ~ non-zero field
double ir = (dir.cross(B).norm()) * q / mom;
double s;
if (state.stepping.navDir == forward) {
if (state.stepping.navDir == NavigationDirection::Forward) {
s = state.stepping.stepSize.max();
} else {
s = state.stepping.stepSize.min();
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Propagator/Propagator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ struct PropagatorResult : private detail::Extendable<result_list...> {
///
struct PropagatorPlainOptions {
/// Propagation direction
NavigationDirection direction = forward;
NavigationDirection direction = NavigationDirection::Forward;

/// The |pdg| code for (eventual) material integration - pion default
int absPdgCode = 211;
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Propagator/StandardAborters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace Acts {
/// @brief TargetOptions struct for geometry interface
struct TargetOptions {
/// Navigation direction
NavigationDirection navDir = forward;
NavigationDirection navDir = NavigationDirection::Forward;

/// Target Boundary check directive - always false here
BoundaryCheck boundaryCheck = false;
Expand Down
7 changes: 4 additions & 3 deletions Core/include/Acts/Propagator/StraightLineStepper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class StraightLineStepper {
explicit State(const GeometryContext& gctx,
const MagneticFieldContext& mctx,
const SingleBoundTrackParameters<charge_t>& par,
NavigationDirection ndir = forward,
NavigationDirection ndir = NavigationDirection::Forward,
double ssize = std::numeric_limits<double>::max(),
double stolerance = s_onSurfaceTolerance)
: q(par.charge()),
Expand Down Expand Up @@ -136,7 +136,7 @@ class StraightLineStepper {
State makeState(std::reference_wrapper<const GeometryContext> gctx,
std::reference_wrapper<const MagneticFieldContext> mctx,
const SingleBoundTrackParameters<charge_t>& par,
NavigationDirection ndir = forward,
NavigationDirection ndir = NavigationDirection::Forward,
double ssize = std::numeric_limits<double>::max(),
double stolerance = s_onSurfaceTolerance) const {
return State{gctx, mctx, par, ndir, ssize, stolerance};
Expand All @@ -152,7 +152,8 @@ class StraightLineStepper {
/// @param [in] stepSize Step size
void resetState(
State& state, const BoundVector& boundParams, const BoundSymMatrix& cov,
const Surface& surface, const NavigationDirection navDir = forward,
const Surface& surface,
const NavigationDirection navDir = NavigationDirection::Forward,
const double stepSize = std::numeric_limits<double>::max()) const;

/// Get the field for the stepping, this gives back a zero field
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,11 @@ struct PointwiseMaterialInteraction {
NoiseUpdateMode updateMode = addNoise) {
// in forward(backward) propagation, energy decreases(increases) and
// variances increase(decrease)
const auto nextE = std::sqrt(mass * mass + momentum * momentum) -
std::copysign(Eloss, nav);
const auto nextE =
std::sqrt(mass * mass + momentum * momentum) -
std::copysign(
Eloss,
static_cast<std::underlying_type_t<NavigationDirection>>(nav));
// put particle at rest if energy loss is too large
nextP = (mass < nextE) ? std::sqrt(nextE * nextE - mass * mass) : 0;
// update track parameters and covariance
Expand Down
12 changes: 8 additions & 4 deletions Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,9 @@ class CombinatorialKalmanFilter {
// Reverse navigation direction to start targeting for the rest
// tracks
state.stepping.navDir =
(state.stepping.navDir == backward) ? forward : backward;
(state.stepping.navDir == NavigationDirection::Backward)
? NavigationDirection::Forward
: NavigationDirection::Backward;
// To avoid meaningless navigation target call
state.stepping.stepSize =
ConstrainedStep(state.stepping.navDir *
Expand Down Expand Up @@ -895,8 +897,8 @@ class CombinatorialKalmanFilter {

} else {
// Kalman update
auto updateRes =
m_extensions.updater(gctx, trackState, forward, getDummyLogger());
auto updateRes = m_extensions.updater(
gctx, trackState, NavigationDirection::Forward, getDummyLogger());
if (!updateRes.ok()) {
ACTS_ERROR("Update step failed: " << updateRes.error());
return updateRes.error();
Expand Down Expand Up @@ -1147,7 +1149,9 @@ class CombinatorialKalmanFilter {
"Reverse navigation direction after smoothing for reaching the "
"target surface");
state.stepping.navDir =
(state.stepping.navDir == forward) ? backward : forward;
(state.stepping.navDir == NavigationDirection::Forward)
? NavigationDirection::Backward
: NavigationDirection::Forward;
}
// Reinitialize the stepping jacobian
state.stepping.jacobian = BoundMatrix::Identity();
Expand Down
8 changes: 4 additions & 4 deletions Core/include/Acts/TrackFitting/GainMatrixUpdater.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ class GainMatrixUpdater {
/// @param[in,out] trackState The track state
/// @param[in] direction The navigation direction
/// @param[in] logger Where to write logging information to
Result<void> operator()(const GeometryContext& gctx,
MultiTrajectory::TrackStateProxy trackState,
NavigationDirection direction = forward,
LoggerWrapper logger = getDummyLogger()) const;
Result<void> operator()(
const GeometryContext& gctx, MultiTrajectory::TrackStateProxy trackState,
NavigationDirection direction = NavigationDirection::Forward,
LoggerWrapper logger = getDummyLogger()) const;
};

} // namespace Acts

0 comments on commit 394cfdb

Please sign in to comment.