Skip to content

Commit

Permalink
refactor!: Make Direction constructor explicit
Browse files Browse the repository at this point in the history
This requires changing the static names values to static functions
  • Loading branch information
paulgessinger committed Jan 15, 2025
1 parent c0e65bc commit 19f11be
Show file tree
Hide file tree
Showing 44 changed files with 227 additions and 224 deletions.
33 changes: 19 additions & 14 deletions Core/include/Acts/Definitions/Direction.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,18 @@ class Direction final {
};

public:
static constexpr auto Negative = Value::Negative;
static constexpr auto Positive = Value::Positive;
static constexpr Direction Negative() { return Direction{Value::Negative}; }
static constexpr Direction Positive() { return Direction{Value::Positive}; }

static constexpr auto Backward = Value::Negative;
static constexpr auto Forward = Value::Positive;
static constexpr Direction Backward() { return Direction{Value::Negative}; }
static constexpr Direction Forward() { return Direction{Value::Positive}; }

static constexpr auto OppositeNormal = Value::Negative;
static constexpr auto AlongNormal = Value::Positive;
static constexpr Direction OppositeNormal() {
return Direction{Value::Negative};
}
static constexpr Direction AlongNormal() {
return Direction{Value::Positive};
}

/// This turns a signed value into a direction. Will assert on zero.
///
Expand All @@ -43,7 +47,7 @@ class Direction final {
/// @return a direction enum
static constexpr Direction fromScalar(double scalar) {
assert(scalar != 0);
return scalar >= 0 ? Value::Positive : Value::Negative;
return Direction{scalar >= 0 ? Value::Positive : Value::Negative};
}

/// This turns a signed value into a direction and 0 will be handled as a
Expand All @@ -54,7 +58,7 @@ class Direction final {
///
/// @return a direction enum
static constexpr Direction fromScalarZeroAsPositive(double scalar) {
return scalar >= 0 ? Value::Positive : Value::Negative;
return Direction{scalar >= 0 ? Value::Positive : Value::Negative};
}

/// Convert and index [0,1] to a direction e.g. for sorting in
Expand All @@ -63,9 +67,9 @@ class Direction final {
/// @param index is the direction at input
static constexpr Direction fromIndex(std::size_t index) {
if (index == 0u) {
return Value::Negative;
return Direction{Value::Negative};
}
return Value::Positive;
return Direction{Value::Positive};
}

/// Convert dir to index [0,1] which allows to store direction dependent
Expand All @@ -88,16 +92,17 @@ class Direction final {
///
/// @return an opposite direction
constexpr Direction invert() const {
return (m_value == Value::Positive) ? Value::Negative : Value::Positive;
return Direction{m_value == Value::Positive ? Value::Negative
: Value::Positive};
}

std::string toString() const;

constexpr Direction() = default;
constexpr Direction(Value value) : m_value(value) {}
explicit constexpr Direction(Value value) : m_value(value) {}

constexpr bool operator==(Direction other) const {
return m_value == other.m_value;
friend constexpr bool operator==(Direction lhs, Direction rhs) {
return lhs.m_value == rhs.m_value;
}

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ struct CorrectedFreeToBoundTransformer {
std::optional<std::tuple<BoundVector, BoundSquareMatrix>> operator()(
const FreeVector& freeParams, const FreeSquareMatrix& freeCovariance,
const Surface& surface, const GeometryContext& geoContext,
Direction navDir = Direction::Forward,
Direction navDir = Direction::Forward(),
const Logger& logger = getDummyLogger()) const;

private:
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 @@ -161,7 +161,7 @@ inline const RegularSurface& BoundarySurfaceT<volume_t>::surfaceRepresentation()
template <class volume_t>
void BoundarySurfaceT<volume_t>::attachVolume(const volume_t* volume,
Direction dir) {
if (dir == Direction::Backward) {
if (dir == Direction::Backward()) {
m_oppositeVolume = volume;
} else {
m_alongVolume = volume;
Expand All @@ -171,7 +171,7 @@ void BoundarySurfaceT<volume_t>::attachVolume(const volume_t* volume,
template <class volume_t>
void BoundarySurfaceT<volume_t>::attachVolumeArray(
const std::shared_ptr<const VolumeArray> volumes, Direction dir) {
if (dir == Direction::Backward) {
if (dir == Direction::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 @@ -137,9 +137,9 @@ inline double ISurfaceMaterial::factor(Direction pDir,
if (mStage == Acts::MaterialUpdateStage::FullUpdate) {
return 1.;
} else if (mStage == Acts::MaterialUpdateStage::PreUpdate) {
return pDir == Direction::Negative ? m_splitFactor : 1 - m_splitFactor;
return pDir == Direction::Negative() ? m_splitFactor : 1 - m_splitFactor;
} else /*if (mStage == Acts::MaterialUpdateStage::PostUpdate)*/ {
return pDir == Direction::Positive ? m_splitFactor : 1 - m_splitFactor;
return pDir == Direction::Positive() ? m_splitFactor : 1 - m_splitFactor;
}
}

Expand Down
6 changes: 3 additions & 3 deletions Core/include/Acts/Propagator/DirectNavigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class DirectNavigator {
}

void nextSurface(Direction direction) {
if (direction == Direction::Forward) {
if (direction == Direction::Forward()) {
++surfaceIndex;
} else {
--surfaceIndex;
Expand All @@ -95,15 +95,15 @@ class DirectNavigator {
}

int remainingSurfaces(Direction direction) const {
if (direction == Direction::Forward) {
if (direction == Direction::Forward()) {
return options.surfaces.size() - surfaceIndex;
}
return surfaceIndex + 1;
}

void resetSurfaceIndex(Direction direction) {
surfaceIndex =
direction == Direction::Forward ? 0 : options.surfaces.size() - 1;
direction == Direction::Forward() ? 0 : options.surfaces.size() - 1;
}
};

Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Propagator/PropagatorOptions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace detail {
/// @brief Holds the generic pure propagator options
struct PurePropagatorPlainOptions {
/// Propagation direction
Direction direction = Direction::Forward;
Direction direction = Direction::Forward();

/// Maximum number of steps for one propagate call
unsigned int maxSteps = 1000;
Expand Down
4 changes: 2 additions & 2 deletions Core/include/Acts/TrackFitting/detail/GsfActor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ struct GsfActor {
auto new_pars = old_bound.parameters();

const auto delta_p = [&]() {
if (state.options.direction == Direction::Forward) {
if (state.options.direction == Direction::Forward()) {
return p_prev * (gaussian.mean - 1.);
} else {
return p_prev * (1. / gaussian.mean - 1.);
Expand All @@ -431,7 +431,7 @@ struct GsfActor {
auto new_cov = old_bound.covariance().value();

const auto varInvP = [&]() {
if (state.options.direction == Direction::Forward) {
if (state.options.direction == Direction::Forward()) {
const auto f = 1. / (p_prev * gaussian.mean);
return f * f * gaussian.var;
} else {
Expand Down
8 changes: 3 additions & 5 deletions Core/src/Detector/detail/CuboidalDetectorHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ Acts::Experimental::detail::CuboidalDetectorHelper::connect(
// Assign the portal direction
// in a consistent way
Acts::Direction dir =
(index % 2 == 0) ? Direction::Forward : Direction::Backward;
(index % 2 == 0) ? Direction::Forward() : Direction::Backward();

// Make the stitch boundaries
pReplacements.push_back(PortalReplacement(
Expand Down Expand Up @@ -283,13 +283,11 @@ Acts::Experimental::detail::CuboidalDetectorHelper::connect(

std::shared_ptr<Portal> sPortal = formerContainer.find(startIndex)->second;
auto sAttachedVolumes =
sPortal
->attachedDetectorVolumes()[Direction(Direction::Backward).index()];
sPortal->attachedDetectorVolumes()[Direction::Backward().index()];

std::shared_ptr<Portal> ePortal = currentContainer.find(endIndex)->second;
auto eAttachedVolumes =
ePortal
->attachedDetectorVolumes()[Direction(Direction::Forward).index()];
ePortal->attachedDetectorVolumes()[Direction::Forward().index()];

auto fusedPortal = Portal::fuse(sPortal, ePortal);

Expand Down
46 changes: 20 additions & 26 deletions Core/src/Detector/detail/CylindricalDetectorHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,8 @@ Acts::Experimental::detail::CylindricalDetectorHelper::connectInR(
std::vector<PortalReplacement> pReplacements = {};

// Disc assignments are forward for negative disc, backward for positive
std::vector<Acts::Direction> discDirs = {Acts::Direction::Forward,
Acts::Direction::Backward};
std::vector<Acts::Direction> discDirs = {Acts::Direction::Forward(),
Acts::Direction::Backward()};
for (const auto [iu, idir] : enumerate(discDirs)) {
if (selectedOnly.empty() || rangeContainsValue(selectedOnly, iu)) {
const Surface& refSurface = volumes[0u]->portals()[iu]->surface();
Expand All @@ -386,8 +386,8 @@ Acts::Experimental::detail::CylindricalDetectorHelper::connectInR(
if (sectorsPresent) {
ACTS_VERBOSE("Sector planes are present, they need replacement.");
// Sector assignments are forward backward
std::vector<Acts::Direction> sectorDirs = {Acts::Direction::Forward,
Acts::Direction::Backward};
std::vector<Acts::Direction> sectorDirs = {Acts::Direction::Forward(),
Acts::Direction::Backward()};
Acts::Vector3 vCenter = volumes[0u]->transform(gctx).translation();
for (const auto [iu, idir] : enumerate(sectorDirs)) {
// (iu + 4u) corresponds to the indices of the phi-low and phi-high sector
Expand Down Expand Up @@ -564,12 +564,12 @@ Acts::Experimental::detail::CylindricalDetectorHelper::connectInZ(
std::vector<PortalReplacement> pReplacements = {};

// Disc assignments are forward for negative disc, backward for positive
std::vector<Acts::Direction> cylinderDirs = {Acts::Direction::Backward};
std::vector<Acts::Direction> cylinderDirs = {Acts::Direction::Backward()};
// Cylinder radii
std::vector<double> cylinderR = {maxR};
if (innerPresent) {
ACTS_VERBOSE("Inner surface present, tube geometry detected.");
cylinderDirs.push_back(Direction::Forward);
cylinderDirs.push_back(Direction::Forward());
cylinderR.push_back(minR);
} else {
ACTS_VERBOSE("No inner surface present, solid cylinder geometry detected.");
Expand All @@ -589,8 +589,8 @@ Acts::Experimental::detail::CylindricalDetectorHelper::connectInZ(
if (sectorsPresent) {
ACTS_VERBOSE("Sector planes are present, they need replacement.");
// Sector assignmenta are forward backward
std::vector<Acts::Direction> sectorDirs = {Acts::Direction::Forward,
Acts::Direction::Backward};
std::vector<Acts::Direction> sectorDirs = {Acts::Direction::Forward(),
Acts::Direction::Backward()};
for (const auto [iu, idir] : enumerate(sectorDirs)) {
// Access with 3u or 4u but always write 4u (to be caught later)
if (selectedOnly.empty() || rangeContainsValue(selectedOnly, iu + 4u)) {
Expand Down Expand Up @@ -710,21 +710,21 @@ Acts::Experimental::detail::CylindricalDetectorHelper::connectInPhi(
refTransform,
{refValues[CylinderVolumeBounds::BoundValues::eMinR],
refValues[CylinderVolumeBounds::BoundValues::eMaxR]},
phiBoundaries, 0u, Acts::Direction::Forward));
phiBoundaries, 0u, Acts::Direction::Forward()));

// Positive disc
pReplacements.push_back(createDiscReplacement(
refTransform,
{refValues[CylinderVolumeBounds::BoundValues::eMinR],
refValues[CylinderVolumeBounds::BoundValues::eMaxR]},
phiBoundaries, 1u, Acts::Direction::Backward));
phiBoundaries, 1u, Acts::Direction::Backward()));

// Outside cylinder
pReplacements.push_back(createCylinderReplacement(
refTransform, refValues[CylinderVolumeBounds::BoundValues::eMaxR],
{-refValues[CylinderVolumeBounds::BoundValues::eHalfLengthZ],
refValues[CylinderVolumeBounds::BoundValues::eHalfLengthZ]},
phiBoundaries, 2u, Acts::Direction::Backward));
phiBoundaries, 2u, Acts::Direction::Backward()));

// If the volume has a different inner radius than 0, it MUST have
// an inner cylinder
Expand All @@ -734,7 +734,7 @@ Acts::Experimental::detail::CylindricalDetectorHelper::connectInPhi(
refTransform, refValues[CylinderVolumeBounds::BoundValues::eMinR],
{-refValues[CylinderVolumeBounds::BoundValues::eHalfLengthZ],
refValues[CylinderVolumeBounds::BoundValues::eHalfLengthZ]},
phiBoundaries, 3u, Acts::Direction::Forward));
phiBoundaries, 3u, Acts::Direction::Forward()));
}

// Attach the new volume multi links
Expand Down Expand Up @@ -822,7 +822,7 @@ Acts::Experimental::detail::CylindricalDetectorHelper::wrapInZR(
std::vector<PortalReplacement> pReplacements;
pReplacements.push_back(createCylinderReplacement(
volumes[0u]->transform(gctx), innerR, {-HlZ, -hlZ, hlZ, HlZ},
{-std::numbers::pi, std::numbers::pi}, 3u, Direction::Forward));
{-std::numbers::pi, std::numbers::pi}, 3u, Direction::Forward()));
std::vector<std::shared_ptr<DetectorVolume>> zVolumes = {
volumes[1u], volumes[0u], volumes[1u]};
// Attach the new volume multi links
Expand Down Expand Up @@ -874,12 +874,10 @@ Acts::Experimental::detail::CylindricalDetectorHelper::connectInR(
std::shared_ptr<Portal> innerCylinder = containers[ic - 1].find(2u)->second;
// Direction is explicitly addressed with a direction index
auto innerAttachedVolumes =
innerCylinder
->attachedDetectorVolumes()[Direction(Direction::Backward).index()];
innerCylinder->attachedDetectorVolumes()[Direction::Backward().index()];
std::shared_ptr<Portal> outerCylinder = containers[ic].find(3u)->second;
auto outerAttachedVolume =
outerCylinder
->attachedDetectorVolumes()[Direction(Direction::Forward).index()];
outerCylinder->attachedDetectorVolumes()[Direction::Forward().index()];
auto fusedCylinder = Portal::fuse(innerCylinder, outerCylinder);

// Update the attached volumes with the new portal
Expand Down Expand Up @@ -944,12 +942,11 @@ Acts::Experimental::detail::CylindricalDetectorHelper::connectInZ(
// Container attachment positive Disc of lower, negative Disc at higher
std::shared_ptr<Portal> pDisc = formerContainer.find(1u)->second;
auto pAttachedVolumes =
pDisc
->attachedDetectorVolumes()[Direction(Direction::Backward).index()];
pDisc->attachedDetectorVolumes()[Direction::Backward().index()];

std::shared_ptr<Portal> nDisc = currentContainer.find(0u)->second;
auto nAttachedVolumes =
nDisc->attachedDetectorVolumes()[Direction(Direction::Forward).index()];
nDisc->attachedDetectorVolumes()[Direction::Forward().index()];

auto fusedDisc = Portal::fuse(pDisc, nDisc);

Expand Down Expand Up @@ -1058,8 +1055,7 @@ Acts::Experimental::detail::CylindricalDetectorHelper::wrapInZR(
// Fuse outer cover of first with inner cylinder of wrapping volume
auto& innerCover = innerContainer[2u];
auto innerAttachedVolumes =
innerCover
->attachedDetectorVolumes()[Direction(Direction::Backward).index()];
innerCover->attachedDetectorVolumes()[Direction::Backward().index()];
auto& innerTube = wrappingVolume->portalPtrs()[3u];
auto fusedCover = Portal::fuse(innerCover, innerTube);

Expand All @@ -1074,8 +1070,7 @@ Acts::Experimental::detail::CylindricalDetectorHelper::wrapInZR(
auto& firstDiscN = innerContainer[0u];

auto firstNAttachedVolumes =
firstDiscN
->attachedDetectorVolumes()[Direction(Direction::Forward).index()];
firstDiscN->attachedDetectorVolumes()[Direction::Forward().index()];

auto& secondDiscN = wrappingVolume->portalPtrs()[4u];
auto fusedDiscN = Portal::fuse(firstDiscN, secondDiscN);
Expand All @@ -1089,8 +1084,7 @@ Acts::Experimental::detail::CylindricalDetectorHelper::wrapInZR(
// Stich sides - positive
auto& firstDiscP = innerContainer[1u];
auto firstPAttachedVolumes =
firstDiscP
->attachedDetectorVolumes()[Direction(Direction::Backward).index()];
firstDiscP->attachedDetectorVolumes()[Direction::Backward().index()];

auto& secondDiscP = wrappingVolume->portalPtrs()[5u];
auto fusedDiscP = Portal::fuse(firstDiscP, secondDiscP);
Expand Down
Loading

0 comments on commit 19f11be

Please sign in to comment.