Skip to content

Commit

Permalink
refactor: Remove constexpr from MultiTrajectory functions (#3073)
Browse files Browse the repository at this point in the history 
I believe the `constexpr` cannot be guaranteed with C++17 for the vector backend and even with C++20 it would limit other backends to be `constexpr`.

From https://en.cppreference.com/w/cpp/language/constexpr
> The constexpr specifier declares that it is possible to evaluate the value of the function or variable at compile time. Such variables and functions can then be used where only compile time [constant expressions](https://en.cppreference.com/w/cpp/language/constant_expression) are allowed (provided that appropriate function arguments are given).

If not all backends can be guaranteed to be `constexpr`, the frontend cannot be `constexpr`.
  • Loading branch information
@andiwand
andiwand authored Apr 6, 2024
1 parent 4935604 commit 1374baa
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 39 deletions.
60 changes: 27 additions & 33 deletions Core/include/Acts/EventData/MultiTrajectory.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ class MultiTrajectory {

/// Helper function to check if a component exists IF it is an optional one.
/// Used in assertions
constexpr bool checkOptional(HashedString key, IndexType istate) const {
bool checkOptional(HashedString key, IndexType istate) const {
using namespace Acts::HashedStringLiteral;
switch (key) {
case "predicted"_hash:
Expand Down Expand Up @@ -240,9 +240,8 @@ class MultiTrajectory {
/// @param iprevious index of the previous state, kInvalid if first
/// @return Index of the newly added track state
template <bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
constexpr IndexType addTrackState(
TrackStatePropMask mask = TrackStatePropMask::All,
IndexType iprevious = kInvalid) {
IndexType addTrackState(TrackStatePropMask mask = TrackStatePropMask::All,
IndexType iprevious = kInvalid) {
return self().addTrackState_impl(mask, iprevious);
}

Expand Down Expand Up @@ -388,29 +387,27 @@ class MultiTrajectory {
/// compatibility with backends.
/// @note Only available if the MultiTrajectory is not read-only
template <typename T, bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
constexpr void addColumn(const std::string& key) {
void addColumn(const std::string& key) {
self().template addColumn_impl<T>(key);
}

/// Check if a column with a key @p key exists.
/// @param key Key to check for a column with
/// @return True if the column exists, false if not.
constexpr bool hasColumn(HashedString key) const {
return self().hasColumn_impl(key);
}
bool hasColumn(HashedString key) const { return self().hasColumn_impl(key); }

/// @}

/// Clear the @c MultiTrajectory. Leaves the underlying storage untouched
/// @note Only available if the MultiTrajectory is not read-only
template <bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
constexpr void clear() {
void clear() {
self().clear_impl();
}

/// Returns the number of track states contained
/// @return The number of track states
constexpr IndexType size() const { return self().size_impl(); }
IndexType size() const { return self().size_impl(); }

protected:
// These are internal helper functions which the @c TrackStateProxy class talks to
Expand All @@ -419,7 +416,7 @@ class MultiTrajectory {
/// @param key The key for which to check
/// @param istate The track state index to check
/// @return True if the component exists, false if not
constexpr bool has(HashedString key, IndexType istate) const {
bool has(HashedString key, IndexType istate) const {
return self().has_impl(key, istate);
}

Expand All @@ -428,55 +425,52 @@ class MultiTrajectory {
/// @param istate The track state index to check
/// @return True if the component exists, false if not
template <HashedString key>
constexpr bool has(IndexType istate) const {
bool has(IndexType istate) const {
return self().has_impl(key, istate);
}

/// Retrieve a parameter proxy instance for parameters at a given index
/// @param parIdx Index into the parameter column
/// @return Mutable proxy
template <bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
constexpr typename TrackStateProxy::Parameters parameters(IndexType parIdx) {
typename TrackStateProxy::Parameters parameters(IndexType parIdx) {
return self().parameters_impl(parIdx);
}

/// Retrieve a parameter proxy instance for parameters at a given index
/// @param parIdx Index into the parameter column
/// @return Const proxy
constexpr typename ConstTrackStateProxy::Parameters parameters(
IndexType parIdx) const {
typename ConstTrackStateProxy::Parameters parameters(IndexType parIdx) const {
return self().parameters_impl(parIdx);
}

/// Retrieve a covariance proxy instance for a covariance at a given index
/// @param covIdx Index into the covariance column
/// @return Mutable proxy
template <bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
constexpr typename TrackStateProxy::Covariance covariance(IndexType covIdx) {
typename TrackStateProxy::Covariance covariance(IndexType covIdx) {
return self().covariance_impl(covIdx);
}

/// Retrieve a covariance proxy instance for a covariance at a given index
/// @param covIdx Index into the covariance column
/// @return Const proxy
constexpr typename ConstTrackStateProxy::Covariance covariance(
IndexType covIdx) const {
typename ConstTrackStateProxy::Covariance covariance(IndexType covIdx) const {
return self().covariance_impl(covIdx);
}

/// Retrieve a jacobian proxy instance for a jacobian at a given index
/// @param jacIdx Index into the jacobian column
/// @return Mutable proxy
template <bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
constexpr typename TrackStateProxy::Covariance jacobian(IndexType jacIdx) {
typename TrackStateProxy::Covariance jacobian(IndexType jacIdx) {
return self().jacobian_impl(jacIdx);
}

/// Retrieve a jacobian proxy instance for a jacobian at a given index
/// @param jacIdx Index into the jacobian column
/// @return Const proxy
constexpr typename ConstTrackStateProxy::Covariance jacobian(
IndexType jacIdx) const {
typename ConstTrackStateProxy::Covariance jacobian(IndexType jacIdx) const {
return self().jacobian_impl(jacIdx);
}

Expand All @@ -486,7 +480,7 @@ class MultiTrajectory {
/// @return Mutable proxy
template <std::size_t measdim, bool RO = ReadOnly,
typename = std::enable_if_t<!RO>>
constexpr typename TrackStateProxy::template Measurement<measdim> measurement(
typename TrackStateProxy::template Measurement<measdim> measurement(
IndexType measIdx) {
return self().template measurement_impl<measdim>(measIdx);
}
Expand All @@ -496,8 +490,8 @@ class MultiTrajectory {
/// @param measIdx Index into the measurement column
/// @return Const proxy
template <std::size_t measdim>
constexpr typename ConstTrackStateProxy::template Measurement<measdim>
measurement(IndexType measIdx) const {
typename ConstTrackStateProxy::template Measurement<measdim> measurement(
IndexType measIdx) const {
return self().template measurement_impl<measdim>(measIdx);
}

Expand All @@ -508,7 +502,7 @@ class MultiTrajectory {
/// @return Mutable proxy
template <std::size_t measdim, bool RO = ReadOnly,
typename = std::enable_if_t<!RO>>
constexpr typename TrackStateProxy::template MeasurementCovariance<measdim>
typename TrackStateProxy::template MeasurementCovariance<measdim>
measurementCovariance(IndexType covIdx) {
return self().template measurementCovariance_impl<measdim>(covIdx);
}
Expand Down Expand Up @@ -542,17 +536,17 @@ class MultiTrajectory {
/// @note The track states both need to be stored in the
/// same @c MultiTrajectory instance
template <bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
constexpr void shareFrom(IndexType iself, IndexType iother,
TrackStatePropMask shareSource,
TrackStatePropMask shareTarget) {
void shareFrom(IndexType iself, IndexType iother,
TrackStatePropMask shareSource,
TrackStatePropMask shareTarget) {
self().shareFrom_impl(iself, iother, shareSource, shareTarget);
}

/// Unset an optional track state component
/// @param target The component to unset
/// @param istate The track state index to operate on
template <bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
constexpr void unset(TrackStatePropMask target, IndexType istate) {
void unset(TrackStatePropMask target, IndexType istate) {
self().unset_impl(target, istate);
}

Expand All @@ -572,7 +566,7 @@ class MultiTrajectory {
/// @return Mutable reference to the component given by @p key
template <typename T, HashedString key, bool RO = ReadOnly,
typename = std::enable_if_t<!RO>>
constexpr T& component(IndexType istate) {
T& component(IndexType istate) {
assert(checkOptional(key, istate));
return *std::any_cast<T*>(self().component_impl(key, istate));
}
Expand All @@ -583,7 +577,7 @@ class MultiTrajectory {
/// @param istate The track state index to operate on
/// @return Mutable reference to the component given by @p key
template <typename T, bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
constexpr T& component(HashedString key, IndexType istate) {
T& component(HashedString key, IndexType istate) {
assert(checkOptional(key, istate));
return *std::any_cast<T*>(self().component_impl(key, istate));
}
Expand All @@ -594,7 +588,7 @@ class MultiTrajectory {
/// @param istate The track state index to operate on
/// @return Const reference to the component given by @p key
template <typename T, HashedString key>
constexpr const T& component(IndexType istate) const {
const T& component(IndexType istate) const {
assert(checkOptional(key, istate));
return *std::any_cast<const T*>(self().component_impl(key, istate));
}
Expand All @@ -605,7 +599,7 @@ class MultiTrajectory {
/// @param istate The track state index to operate on
/// @return Const reference to the component given by @p key
template <typename T>
constexpr const T& component(HashedString key, IndexType istate) const {
const T& component(HashedString key, IndexType istate) const {
assert(checkOptional(key, istate));
return *std::any_cast<const T*>(self().component_impl(key, istate));
}
Expand Down
12 changes: 6 additions & 6 deletions Core/include/Acts/EventData/VectorMultiTrajectory.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ class VectorMultiTrajectoryBase {
}

template <typename T>
static constexpr bool hasColumn_impl(T& instance, HashedString key) {
static bool hasColumn_impl(T& instance, HashedString key) {
using namespace Acts::HashedStringLiteral;
switch (key) {
case "predicted"_hash:
Expand Down Expand Up @@ -434,7 +434,7 @@ class VectorMultiTrajectory final

void unset_impl(TrackStatePropMask target, IndexType istate);

constexpr bool has_impl(HashedString key, IndexType istate) const {
bool has_impl(HashedString key, IndexType istate) const {
return detail_vmt::VectorMultiTrajectoryBase::has_impl(*this, key, istate);
}

Expand All @@ -455,12 +455,12 @@ class VectorMultiTrajectory final
}

template <typename T>
constexpr void addColumn_impl(const std::string& key) {
void addColumn_impl(const std::string& key) {
Acts::HashedString hashedKey = hashString(key);
m_dynamic.insert({hashedKey, std::make_unique<detail::DynamicColumn<T>>()});
}

constexpr bool hasColumn_impl(HashedString key) const {
bool hasColumn_impl(HashedString key) const {
return detail_vmt::VectorMultiTrajectoryBase::hasColumn_impl(*this, key);
}

Expand Down Expand Up @@ -561,7 +561,7 @@ class ConstVectorMultiTrajectory final
&m_measCov[offset]};
}

constexpr bool has_impl(HashedString key, IndexType istate) const {
bool has_impl(HashedString key, IndexType istate) const {
return detail_vmt::VectorMultiTrajectoryBase::has_impl(*this, key, istate);
}

Expand All @@ -574,7 +574,7 @@ class ConstVectorMultiTrajectory final
*this, key, istate);
}

constexpr bool hasColumn_impl(HashedString key) const {
bool hasColumn_impl(HashedString key) const {
return detail_vmt::VectorMultiTrajectoryBase::hasColumn_impl(*this, key);
}

Expand Down

0 comments on commit 1374baa

Please sign in to comment.