Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: Make constructors explicit #3975

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions Core/include/Acts/EventData/Charge.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ struct Neutral {
/// Construct and verify the input charge magnitude (in debug builds).
///
/// This constructor is only provided to allow consistent construction.
constexpr Neutral(float absQ) noexcept {
constexpr explicit Neutral(float absQ) noexcept {
assert((absQ == 0) && "Input charge must be zero");
(void)absQ;
}
Expand Down Expand Up @@ -102,7 +102,7 @@ struct SinglyCharged {
/// Construct and verify the input charge magnitude (in debug builds).
///
/// This constructor is only provided to allow consistent construction.
constexpr SinglyCharged(float absQ) noexcept {
constexpr explicit SinglyCharged(float absQ) noexcept {
assert((absQ == UnitConstants::e) && "Input charge magnitude must be e");
(void)absQ;
}
Expand Down Expand Up @@ -141,10 +141,10 @@ static_assert(ChargeConcept<SinglyCharged>,
class NonNeutralCharge {
public:
/// Construct with the magnitude of the input charge.
constexpr NonNeutralCharge(float absQ) noexcept : m_absQ{absQ} {
constexpr explicit NonNeutralCharge(float absQ) noexcept : m_absQ{absQ} {
assert((0 < absQ) && "Input charge magnitude must be positive");
}
constexpr NonNeutralCharge(SinglyCharged /*unused*/) noexcept
constexpr explicit NonNeutralCharge(SinglyCharged /*unused*/) noexcept
: m_absQ{UnitConstants::e} {}

constexpr float absQ() const noexcept { return m_absQ; }
Expand Down Expand Up @@ -182,12 +182,12 @@ static_assert(ChargeConcept<NonNeutralCharge>,
class AnyCharge {
public:
/// Construct with the magnitude of the input charge.
constexpr AnyCharge(float absQ) noexcept : m_absQ{absQ} {
constexpr explicit AnyCharge(float absQ) noexcept : m_absQ{absQ} {
assert((0 <= absQ) && "Input charge magnitude must be zero or positive");
}
constexpr AnyCharge(SinglyCharged /*unused*/) noexcept
constexpr explicit AnyCharge(SinglyCharged /*unused*/) noexcept
: m_absQ{UnitConstants::e} {}
constexpr AnyCharge(Neutral /*unused*/) noexcept {}
constexpr explicit AnyCharge(Neutral /*unused*/) noexcept {}

constexpr float absQ() const noexcept { return m_absQ; }

Expand Down
8 changes: 7 additions & 1 deletion Core/include/Acts/EventData/GenericBoundTrackParameters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#pragma once

#include "Acts/Definitions/Tolerance.hpp"
#include "Acts/EventData/ParticleHypothesis.hpp"
#include "Acts/EventData/TrackParameterHelpers.hpp"
#include "Acts/EventData/TransformationHelpers.hpp"
#include "Acts/EventData/detail/PrintParameters.hpp"
Expand Down Expand Up @@ -69,12 +70,17 @@ class GenericBoundTrackParameters {

/// Converts a bound track parameter with a different hypothesis.
template <typename other_particle_hypothesis_t>
GenericBoundTrackParameters(
explicit GenericBoundTrackParameters(
const GenericBoundTrackParameters<other_particle_hypothesis_t>& other)
: GenericBoundTrackParameters(other.referenceSurface().getSharedPtr(),
other.parameters(), other.covariance(),
other.particleHypothesis()) {}

/// Convert this track parameter object to the general type-erased one
GenericBoundTrackParameters<Acts::ParticleHypothesis> toBound() const {
return GenericBoundTrackParameters<Acts::ParticleHypothesis>{*this};
}

/// Factory to construct from four-position, direction, absolute momentum, and
/// charge.
///
Expand Down
3 changes: 3 additions & 0 deletions Core/include/Acts/EventData/MultiComponentTrackParameters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ class MultiComponentBoundTrackParameters {
MultiComponentBoundTrackParameters& operator=(
MultiComponentBoundTrackParameters&&) = default;

/// Comply with bound convertible, in this case return a copy
MultiComponentBoundTrackParameters toBound() const { return *this; }

/// Access the parameters
const auto& components() const { return m_components; }

Expand Down
22 changes: 13 additions & 9 deletions Core/include/Acts/EventData/ParticleHypothesis.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class SinglyChargedParticleHypothesis
public:
constexpr SinglyChargedParticleHypothesis(PdgParticle absPdg, float mass)
: GenericParticleHypothesis(absPdg, mass, {}) {}
SinglyChargedParticleHypothesis(PdgParticle absPdg)

explicit SinglyChargedParticleHypothesis(PdgParticle absPdg)
: GenericParticleHypothesis(absPdg) {}

template <typename other_charge_t>
Expand Down Expand Up @@ -70,7 +71,7 @@ class NeutralParticleHypothesis : public GenericParticleHypothesis<Neutral> {
public:
constexpr NeutralParticleHypothesis(PdgParticle absPdg, float mass)
: GenericParticleHypothesis(absPdg, mass, {}) {}
NeutralParticleHypothesis(PdgParticle absPdg)
explicit NeutralParticleHypothesis(PdgParticle absPdg)
: GenericParticleHypothesis(absPdg) {}

template <typename other_charge_t>
Expand Down Expand Up @@ -102,7 +103,7 @@ class NonNeutralChargedParticleHypothesis
constexpr NonNeutralChargedParticleHypothesis(PdgParticle absPdg, float mass,
NonNeutralCharge chargeType)
: GenericParticleHypothesis(absPdg, mass, chargeType) {}
NonNeutralChargedParticleHypothesis(PdgParticle absPdg)
explicit NonNeutralChargedParticleHypothesis(PdgParticle absPdg)
: GenericParticleHypothesis(absPdg) {}

template <typename other_charge_t>
Expand All @@ -127,16 +128,17 @@ class NonNeutralChargedParticleHypothesis
}

static NonNeutralChargedParticleHypothesis pionLike(float absQ) {
return NonNeutralChargedParticleHypothesis(pion().absolutePdg(),
pion().mass(), absQ);
return NonNeutralChargedParticleHypothesis(
pion().absolutePdg(), pion().mass(), NonNeutralCharge{absQ});
}

static NonNeutralChargedParticleHypothesis chargedGeantino() {
static const auto cache = chargedGeantino(Acts::UnitConstants::e);
return cache;
}
static NonNeutralChargedParticleHypothesis chargedGeantino(float absQ) {
return NonNeutralChargedParticleHypothesis(PdgParticle::eInvalid, 0, absQ);
return NonNeutralChargedParticleHypothesis(PdgParticle::eInvalid, 0,
NonNeutralCharge{absQ});
}
};

Expand All @@ -148,7 +150,8 @@ class ParticleHypothesis : public GenericParticleHypothesis<AnyCharge> {
constexpr ParticleHypothesis(PdgParticle absPdg, float mass,
AnyCharge chargeType)
: GenericParticleHypothesis(absPdg, mass, chargeType) {}
ParticleHypothesis(PdgParticle absPdg) : GenericParticleHypothesis(absPdg) {}
explicit ParticleHypothesis(PdgParticle absPdg)
: GenericParticleHypothesis(absPdg) {}

template <typename other_charge_t>
constexpr ParticleHypothesis(
Expand Down Expand Up @@ -179,7 +182,8 @@ class ParticleHypothesis : public GenericParticleHypothesis<AnyCharge> {
}

static ParticleHypothesis pionLike(float absQ) {
return ParticleHypothesis(pion().absolutePdg(), pion().mass(), absQ);
return ParticleHypothesis(pion().absolutePdg(), pion().mass(),
AnyCharge{absQ});
}

static ParticleHypothesis geantino() {
Expand All @@ -190,7 +194,7 @@ class ParticleHypothesis : public GenericParticleHypothesis<AnyCharge> {
return cache;
}
static ParticleHypothesis chargedGeantino(float absQ) {
return ParticleHypothesis(PdgParticle::eInvalid, 0, absQ);
return ParticleHypothesis(PdgParticle::eInvalid, 0, AnyCharge{absQ});
}
};

Expand Down
7 changes: 7 additions & 0 deletions Core/include/Acts/EventData/TrackParametersConcept.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,11 @@ concept BoundTrackParametersConcept =
};
};

namespace Concepts {
template <typename Parameters>
concept BoundConvertibleTrackParameters = requires(const Parameters &p) {
{ p.toBound() } -> BoundTrackParametersConcept;
};
} // namespace Concepts

} // namespace Acts
8 changes: 6 additions & 2 deletions Core/include/Acts/Propagator/Propagator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,9 @@ class Propagator final
template <typename parameters_t, typename propagator_options_t,
typename path_aborter_t = PathLimitReached>
auto makeState(const parameters_t& start,
const propagator_options_t& options) const;
const propagator_options_t& options) const
requires Concepts::BasicTrackParameters<parameters_t> &&
Concepts::BoundConvertibleTrackParameters<parameters_t>;

/// @brief Builds the propagator state object
///
Expand All @@ -322,7 +324,9 @@ class Propagator final
typename target_aborter_t = SurfaceReached,
typename path_aborter_t = PathLimitReached>
auto makeState(const parameters_t& start, const Surface& target,
const propagator_options_t& options) const;
const propagator_options_t& options) const
requires Concepts::BasicTrackParameters<parameters_t> &&
Concepts::BoundConvertibleTrackParameters<parameters_t>;

/// @brief Propagate track parameters
///
Expand Down
14 changes: 10 additions & 4 deletions Core/include/Acts/Propagator/Propagator.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ template <typename S, typename N>
template <typename parameters_t, typename propagator_options_t,
typename path_aborter_t>
auto Acts::Propagator<S, N>::makeState(
const parameters_t& start, const propagator_options_t& options) const {
const parameters_t& start, const propagator_options_t& options) const
requires Concepts::BasicTrackParameters<parameters_t> &&
Concepts::BoundConvertibleTrackParameters<parameters_t>
{
static_assert(BoundTrackParametersConcept<parameters_t>,
"Parameters do not fulfill bound parameters concept.");

Expand All @@ -177,8 +180,8 @@ auto Acts::Propagator<S, N>::makeState(
// Initialize the internal propagator state
StateType state{
eOptions,
m_stepper.makeState(eOptions.geoContext, eOptions.magFieldContext, start,
eOptions.stepping.maxStepSize),
m_stepper.makeState(eOptions.geoContext, eOptions.magFieldContext,
start.toBound(), eOptions.stepping.maxStepSize),
m_navigator.makeState(eOptions.navigation)};

static_assert(
Expand All @@ -196,7 +199,10 @@ template <typename parameters_t, typename propagator_options_t,
typename target_aborter_t, typename path_aborter_t>
auto Acts::Propagator<S, N>::makeState(
const parameters_t& start, const Surface& target,
const propagator_options_t& options) const {
const propagator_options_t& options) const
requires Concepts::BasicTrackParameters<parameters_t> &&
Concepts::BoundConvertibleTrackParameters<parameters_t>
{
static_assert(BoundTrackParametersConcept<parameters_t>,
"Parameters do not fulfill bound parameters concept.");

Expand Down
6 changes: 3 additions & 3 deletions Core/include/Acts/Surfaces/Surface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,20 +89,20 @@ class Surface : public virtual GeometryObject,
///
/// @param transform Transform3 positions the surface in 3D global space
/// @note also acts as default constructor
Surface(const Transform3& transform = Transform3::Identity());
explicit Surface(const Transform3& transform = Transform3::Identity());

/// Copy constructor
///
/// @note copy construction invalidates the association
/// to detector element and layer
///
/// @param other Source surface for copy.
Surface(const Surface& other);
explicit Surface(const Surface& other);

/// Constructor from DetectorElementBase: Element proxy
///
/// @param detelement Detector element which is represented by this surface
Surface(const DetectorElementBase& detelement);
explicit Surface(const DetectorElementBase& detelement);

/// Copy constructor with optional shift
///
Expand Down
4 changes: 2 additions & 2 deletions Core/include/Acts/Utilities/Logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,8 @@ class TimedOutputDecorator final : public OutputDecorator {
///
/// @param [in] wrappee output print policy object to be wrapped
/// @param [in] format format of time stamp (see std::strftime)
TimedOutputDecorator(std::unique_ptr<OutputPrintPolicy> wrappee,
const std::string& format = "%X")
explicit TimedOutputDecorator(std::unique_ptr<OutputPrintPolicy> wrappee,
const std::string& format = "%X")
: OutputDecorator(std::move(wrappee)), m_format(format) {}

/// @brief flush the debug message to the destination stream
Expand Down
10 changes: 7 additions & 3 deletions Examples/Python/src/EventData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,16 @@ void addEventData(Context& ctx) {
auto [m, mex] = ctx.get("main", "examples");

py::class_<Acts::ParticleHypothesis>(m, "ParticleHypothesis")
.def(py::init<PdgParticle, float, float>(), py::arg("pdg"),
py::arg("mass"), py::arg("absCharge"))
.def(py::init([](Acts::PdgParticle absPdg, float mass, float absCharge) {
return Acts::ParticleHypothesis(absPdg, mass,
AnyCharge{absCharge});
}),
py::arg("pdg"), py::arg("mass"), py::arg("absCharge"))
.def(py::init([](std::underlying_type_t<Acts::PdgParticle> absPdg,
float mass, float absCharge) {
return Acts::ParticleHypothesis(
static_cast<Acts::PdgParticle>(absPdg), mass, absCharge);
static_cast<Acts::PdgParticle>(absPdg), mass,
AnyCharge{absCharge});
}),
py::arg("absPdg"), py::arg("mass"), py::arg("absCharge"))
.def("__str__",
Expand Down
4 changes: 3 additions & 1 deletion Fatras/include/ActsFatras/EventData/Particle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,9 @@ class Particle {

/// Particle hypothesis.
Acts::ParticleHypothesis hypothesis() const {
return Acts::ParticleHypothesis(absolutePdg(), mass(), absoluteCharge());
return Acts::ParticleHypothesis(
absolutePdg(), mass(),
Acts::AnyCharge{static_cast<float>(absoluteCharge())});
}
/// Particl qOverP.
double qOverP() const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ class PodioTrackContainerBase {
auto track = instance.m_collection->at(itrack);
const auto& src = track.getParticleHypothesis();
return ParticleHypothesis{static_cast<PdgParticle>(src.absPdg), src.mass,
src.absQ};
AnyCharge{src.absQ}};
}

static void populateSurfaceBuffer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ BOOST_AUTO_TEST_CASE(volume_material_interaction_test) {
// Create a propagator state
State state;
state.stepping.particleHypothesis =
ParticleHypothesis(static_cast<PdgParticle>(11), 10., 9.);
ParticleHypothesis(static_cast<PdgParticle>(11), 10., AnyCharge{9.});
state.stepping.pos = Vector3(1., 2., 3.);
state.stepping.dir = Vector3(4., 5., 6.);
state.stepping.t = 7.;
Expand Down
Loading