Skip to content

Commit

Permalink
Merge branch 'main' into numbers-const
Browse files Browse the repository at this point in the history
  • Loading branch information
AJPfleger authored Oct 29, 2024
2 parents 4062945 + 9c0bc47 commit ab9267b
Show file tree
Hide file tree
Showing 22 changed files with 192 additions and 57 deletions.
7 changes: 5 additions & 2 deletions 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/TrackParameterHelpers.hpp"
#include "Acts/EventData/TransformationHelpers.hpp"
#include "Acts/EventData/detail/PrintParameters.hpp"
#include "Acts/Surfaces/Surface.hpp"
Expand All @@ -18,7 +19,6 @@
#include <cassert>
#include <cmath>
#include <memory>
#include <type_traits>

namespace Acts {

Expand Down Expand Up @@ -61,7 +61,10 @@ class GenericBoundTrackParameters {
m_cov(std::move(cov)),
m_surface(std::move(surface)),
m_particleHypothesis(std::move(particleHypothesis)) {
assert(m_surface);
// TODO set `validateAngleRange` to `true` after fixing caller code
assert(isBoundVectorValid(m_params, false) &&
"Invalid bound parameters vector");
assert(m_surface != nullptr && "Reference surface must not be null");
normalizePhiTheta();
}

Expand Down
11 changes: 8 additions & 3 deletions Core/include/Acts/EventData/GenericFreeTrackParameters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,16 @@
#include "Acts/Definitions/Algebra.hpp"
#include "Acts/Definitions/Common.hpp"
#include "Acts/Definitions/TrackParametrization.hpp"
#include "Acts/EventData/TrackParameterHelpers.hpp"
#include "Acts/EventData/TrackParametersConcept.hpp"
#include "Acts/EventData/TransformationHelpers.hpp"
#include "Acts/EventData/detail/PrintParameters.hpp"
#include "Acts/Utilities/MathHelpers.hpp"
#include "Acts/Utilities/UnitVectors.hpp"
#include "Acts/Utilities/VectorHelpers.hpp"

#include <cassert>
#include <cmath>
#include <optional>
#include <type_traits>

namespace Acts {

Expand Down Expand Up @@ -55,7 +54,9 @@ class GenericFreeTrackParameters {
ParticleHypothesis particleHypothesis)
: m_params(params),
m_cov(std::move(cov)),
m_particleHypothesis(std::move(particleHypothesis)) {}
m_particleHypothesis(std::move(particleHypothesis)) {
assert(isFreeVectorValid(m_params) && "Invalid free parameters vector");
}

/// Construct from four-position, direction, absolute momentum, and charge.
///
Expand All @@ -78,6 +79,8 @@ class GenericFreeTrackParameters {
m_params[eFreeDir1] = dir[eMom1];
m_params[eFreeDir2] = dir[eMom2];
m_params[eFreeQOverP] = qOverP;

assert(isFreeVectorValid(m_params) && "Invalid free parameters vector");
}

/// Construct from four-position, angles, absolute momentum, and charge.
Expand All @@ -103,6 +106,8 @@ class GenericFreeTrackParameters {
m_params[eFreeDir1] = dir[eMom1];
m_params[eFreeDir2] = dir[eMom2];
m_params[eFreeQOverP] = qOverP;

assert(isFreeVectorValid(m_params) && "Invalid free parameters vector");
}

/// Converts a free track parameter with a different hypothesis.
Expand Down
30 changes: 30 additions & 0 deletions Core/include/Acts/EventData/TrackParameterHelpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,38 @@
#include "Acts/Definitions/TrackParametrization.hpp"
#include "Acts/Utilities/detail/periodic.hpp"

#include <limits>

namespace Acts {

/// Check if a bound vector is valid. This checks the following:
/// - All values are finite
/// - (optionally) The phi value is in the range [-pi, pi)
/// - (optionally) The theta value is in the range [0, pi]
///
/// @param v The bound vector to check
/// @param validateAngleRange If true, the phi and theta values are range checked
/// @param epsilon The epsilon to use for the checks
/// @param maxAbsEta The maximum allowed eta value
///
/// @return True if the bound vector is valid
bool isBoundVectorValid(
const BoundVector& v, bool validateAngleRange, double epsilon = 1e-6,
double maxAbsEta = std::numeric_limits<double>::infinity());

/// Check if a free vector is valid. This checks the following:
/// - All values are finite
/// - Direction is normalized
///
/// @param v The free vector to check
/// @param epsilon The epsilon to use for the checks
/// @param maxAbsEta The maximum allowed eta value
///
/// @return True if the free vector is valid
bool isFreeVectorValid(
const FreeVector& v, double epsilon = 1e-6,
double maxAbsEta = std::numeric_limits<double>::infinity());

/// Normalize the bound parameter angles
///
/// @param boundParams The bound parameters to normalize
Expand Down
3 changes: 1 addition & 2 deletions Core/include/Acts/EventData/TrackParametersConcept.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
#include "Acts/Definitions/TrackParametrization.hpp"
#include "Acts/Geometry/GeometryContext.hpp"

#include <concepts>
#include <optional>
#include <type_traits>

namespace Acts {

Expand Down Expand Up @@ -68,4 +66,5 @@ concept BoundTrackParametersConcept =
{ p.position(c) } -> std::same_as<Vector3>;
};
};

} // namespace Acts
8 changes: 8 additions & 0 deletions Core/include/Acts/EventData/detail/GenerateParameters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ struct GenerateBoundParametersOptions {
GenerateQoverPOptions qOverP;
};

inline BoundVector someBoundParametersA() {
return {0.0, 0.0, 0.0, std::numbers::pi / 2, 1.0, 0.0};
}

template <typename generator_t>
inline BoundVector generateBoundParameters(
generator_t& rng, const GenerateBoundParametersOptions& options) {
Expand Down Expand Up @@ -242,6 +246,10 @@ struct GenerateFreeParametersOptions {
GenerateQoverPOptions qOverP;
};

inline FreeVector someFreeParametersA() {
return {0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 1.0};
}

template <typename generator_t>
inline FreeVector generateFreeParameters(
generator_t& rng, const GenerateFreeParametersOptions& options) {
Expand Down
6 changes: 3 additions & 3 deletions Core/include/Acts/EventData/detail/TestTrackState.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ struct TestTrackState {
: surface(
CurvilinearSurface(Vector3::Zero(), Vector3::UnitZ()).surface()),
// set bogus parameters first since they are not default-constructible
predicted(surface, BoundVector::Zero(), std::nullopt,
predicted(surface, someBoundParametersA(), std::nullopt,
ParticleHypothesis::pion()),
filtered(surface, BoundVector::Zero(), std::nullopt,
filtered(surface, someBoundParametersA(), std::nullopt,
ParticleHypothesis::pion()),
smoothed(surface, BoundVector::Zero(), std::nullopt,
smoothed(surface, someBoundParametersA(), std::nullopt,
ParticleHypothesis::pion()),
jacobian(BoundMatrix::Identity()),
chi2(std::chi_squared_distribution<double>(measdim)(rng)),
Expand Down
19 changes: 7 additions & 12 deletions Core/include/Acts/Geometry/NavigationPolicyFactory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,12 @@ class NavigationPolicyFactory {
namespace detail {

template <typename F, typename... Args>
concept NavigationPolicyIsolatedFactoryConcept = requires(F f) {
{
f(std::declval<const GeometryContext&>(),
std::declval<const TrackingVolume&>(), std::declval<const Logger&>(),
std::declval<Args>()...)
} -> std::derived_from<INavigationPolicy>;

requires NavigationPolicyConcept<decltype(f(
std::declval<const GeometryContext&>(),
std::declval<const TrackingVolume&>(), std::declval<const Logger&>(),
std::declval<Args>()...))>;
concept NavigationPolicyIsolatedFactoryConcept = requires(
F f, const GeometryContext& gctx, const TrackingVolume& volume,
const Logger& logger, Args&&... args) {
{ f(gctx, volume, logger, args...) } -> std::derived_from<INavigationPolicy>;

requires NavigationPolicyConcept<decltype(f(gctx, volume, logger, args...))>;

requires(std::is_copy_constructible_v<Args> && ...);
};
Expand Down Expand Up @@ -205,7 +200,7 @@ class NavigationPolicyFactoryImpl<F, Fs...> : public NavigationPolicyFactory {
template <typename...>
friend class NavigationPolicyFactoryImpl;

NavigationPolicyFactoryImpl(std::tuple<F, Fs...>&& factories)
explicit NavigationPolicyFactoryImpl(std::tuple<F, Fs...>&& factories)
: m_factories(std::move(factories)) {}

std::tuple<F, Fs...> m_factories;
Expand Down
4 changes: 2 additions & 2 deletions Core/include/Acts/Geometry/TrackingVolume.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ class TrackingVolume : public Volume {
~TrackingVolume() override;
TrackingVolume(const TrackingVolume&) = delete;
TrackingVolume& operator=(const TrackingVolume&) = delete;
TrackingVolume(TrackingVolume&&);
TrackingVolume& operator=(TrackingVolume&&);
TrackingVolume(TrackingVolume&&) noexcept;
TrackingVolume& operator=(TrackingVolume&&) noexcept;

/// Constructor for a container Volume
/// - vacuum filled volume either as a for other tracking volumes
Expand Down
8 changes: 5 additions & 3 deletions Core/include/Acts/Navigation/INavigationPolicy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ class INavigationPolicy {
public:
/// Noop update function that is suitable as a default for default navigation
/// delegates.
static void noopInitializeCandidates(const NavigationArguments& /*unused*/,
AppendOnlyNavigationStream& /*unused*/,
const Logger& /*unused*/) {}
static void noopInitializeCandidates(
const NavigationArguments& /*unused*/,
const AppendOnlyNavigationStream& /*unused*/, const Logger& /*unused*/) {
// This is a noop
}

/// Virtual destructor so policies can be held through this base class.
virtual ~INavigationPolicy() = default;
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Navigation/MultiNavigationPolicy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class MultiNavigationPolicy final : public MultiNavigationPolicyBase {
public:
/// Constructor from a set of child policies.
/// @param policies The child policies
MultiNavigationPolicy(Policies&&... policies)
explicit MultiNavigationPolicy(Policies&&... policies)
: m_policies{std::move(policies)...} {}

/// Implementation of the connection to a navigation delegate.
Expand Down
7 changes: 4 additions & 3 deletions Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,14 @@ class SurfaceArrayNavigationPolicy : public INavigationPolicy {
friend std::ostream& operator<<(std::ostream& os,
const LayerType& layerType) {
switch (layerType) {
case LayerType::Cylinder:
using enum LayerType;
case Cylinder:
os << "Cylinder";
break;
case LayerType::Disc:
case Disc:
os << "Disc";
break;
case LayerType::Plane:
case Plane:
os << "Plane";
break;
}
Expand Down
4 changes: 2 additions & 2 deletions Core/include/Acts/Utilities/DelegateChainBuilder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class DelegateChainBuilder<R(callable_args...), TypeList<payload_types...>,

template <std::size_t I = 0, typename result_ptr>
static constexpr auto invoke(result_ptr result, const tuple_type* payloads,
callable_args... args) {
callable_args&&... args) {
const auto& callable = findCallable<I, 0, callables...>();

if constexpr (!std::is_same_v<std::tuple_element_t<I, tuple_type>,
Expand Down Expand Up @@ -134,7 +134,7 @@ class DelegateChainBuilder<R(callable_args...), TypeList<payload_types...>,

tuple_type m_payloads{};

auto dispatch(callable_args... args) const {
auto dispatch(callable_args&&... args) const {
if constexpr (std::is_same_v<R, void>) {
invoke(nullptr, &m_payloads, std::forward<callable_args>(args)...);
} else {
Expand Down
1 change: 1 addition & 0 deletions Core/src/EventData/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ target_sources(
TrackStatePropMask.cpp
VectorMultiTrajectory.cpp
VectorTrackContainer.cpp
TrackParameterHelpers.cpp
)
64 changes: 64 additions & 0 deletions Core/src/EventData/TrackParameterHelpers.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// This file is part of the ACTS project.
//
// Copyright (C) 2016 CERN for the benefit of the ACTS project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

#include "Acts/EventData/TrackParameterHelpers.hpp"

#include "Acts/Utilities/AngleHelpers.hpp"
#include "Acts/Utilities/VectorHelpers.hpp"

#include <numbers>

bool Acts::isBoundVectorValid(const BoundVector& v, bool validateAngleRange,
double epsilon, double maxAbsEta) {
constexpr auto pi = std::numbers::pi_v<double>;

bool loc0Valid = std::isfinite(v[eBoundLoc0]);
bool loc1Valid = std::isfinite(v[eBoundLoc1]);
bool phiValid = std::isfinite(v[eBoundPhi]);
bool thetaValid = std::isfinite(v[eBoundTheta]);
bool qOverPValid = std::isfinite(v[eBoundQOverP]);
bool timeValid = std::isfinite(v[eBoundTime]);

if (validateAngleRange) {
phiValid = phiValid && (v[eBoundPhi] + epsilon >= -pi) &&
(v[eBoundPhi] - epsilon < pi);
thetaValid = thetaValid && (v[eBoundTheta] + epsilon >= 0) &&
(v[eBoundTheta] - epsilon <= pi);
}

bool etaValid = true;
if (std::isfinite(maxAbsEta)) {
double eta = AngleHelpers::etaFromTheta(v[eBoundTheta]);
etaValid = std::isfinite(eta) && (std::abs(eta) - epsilon <= maxAbsEta);
}

return loc0Valid && loc1Valid && phiValid && thetaValid && qOverPValid &&
timeValid && etaValid;
}

bool Acts::isFreeVectorValid(const FreeVector& v, double epsilon,
double maxAbsEta) {
bool pos0Valid = std::isfinite(v[eFreePos0]);
bool pos1Valid = std::isfinite(v[eFreePos1]);
bool pos2Valid = std::isfinite(v[eFreePos2]);
bool dir0Valid = std::isfinite(v[eFreeDir0]);
bool dir1Valid = std::isfinite(v[eFreeDir1]);
bool dir2Valid = std::isfinite(v[eFreeDir2]);
bool dirValid = (std::abs(v.segment<3>(eFreeDir0).norm() - 1) - epsilon <= 0);
bool qOverPValid = std::isfinite(v[eFreeQOverP]);
bool timeValid = std::isfinite(v[eFreeTime]);

bool etaValid = true;
if (std::isfinite(maxAbsEta)) {
double eta = VectorHelpers::eta(v.segment<3>(eFreeDir0));
etaValid = std::isfinite(eta) && (std::abs(eta) - epsilon <= maxAbsEta);
}

return pos0Valid && pos1Valid && pos2Valid && dir0Valid && dir1Valid &&
dir2Valid && dirValid && qOverPValid && timeValid && etaValid;
}
4 changes: 2 additions & 2 deletions Core/src/Geometry/TrackingVolume.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ TrackingVolume::TrackingVolume(const Transform3& transform,
{}, volumeName) {}

TrackingVolume::~TrackingVolume() = default;
TrackingVolume::TrackingVolume(TrackingVolume&&) = default;
TrackingVolume& TrackingVolume::operator=(TrackingVolume&&) = default;
TrackingVolume::TrackingVolume(TrackingVolume&&) noexcept = default;
TrackingVolume& TrackingVolume::operator=(TrackingVolume&&) noexcept = default;

const TrackingVolume* TrackingVolume::lowestTrackingVolume(
const GeometryContext& gctx, const Vector3& position,
Expand Down
2 changes: 0 additions & 2 deletions Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ SurfaceArrayNavigationPolicy::SurfaceArrayNavigationPolicy(
auto [binsPhi, binsZ] = config.bins;
m_surfaceArray =
sac.surfaceArrayOnCylinder(gctx, std::move(surfaces), binsPhi, binsZ);
// m_surfaces = sac.createCylinderSurfaces(config.bins.first,
// config.bins.second);
} else if (config.layerType == LayerType::Plane) {
ACTS_ERROR("Plane layers are not yet supported");
throw std::invalid_argument("Plane layers are not yet supported");
Expand Down
4 changes: 2 additions & 2 deletions Examples/Python/src/Navigation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct AnyNavigationPolicyFactory : public Acts::NavigationPolicyFactory {
template <typename Factory = detail::NavigationPolicyFactoryImpl<>,
typename... Policies>
struct NavigationPolicyFactoryT : public AnyNavigationPolicyFactory {
NavigationPolicyFactoryT(Factory impl)
explicit NavigationPolicyFactoryT(Factory impl)
requires(sizeof...(Policies) > 0)
: m_impl(std::move(impl)) {}

Expand Down Expand Up @@ -74,7 +74,7 @@ struct NavigationPolicyFactoryT : public AnyNavigationPolicyFactory {
private:
template <typename T, typename... Args>
std::unique_ptr<AnyNavigationPolicyFactory> add(Args&&... args) {
if constexpr (!((std::is_same_v<T, Policies> || ...))) {
if constexpr (!(std::is_same_v<T, Policies> || ...)) {
auto impl =
std::move(m_impl).template add<T>(std::forward<Args>(args)...);
return std::make_unique<
Expand Down
10 changes: 10 additions & 0 deletions Tests/UnitTests/Core/EventData/TrackParameterHelpersTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@

BOOST_AUTO_TEST_SUITE(TrackParameterHelpers)

BOOST_AUTO_TEST_CASE(isBoundVectorValid) {
BOOST_CHECK(!Acts::isBoundVectorValid({1, 2, 3, 4, 5, 6}, true));
BOOST_CHECK(Acts::isBoundVectorValid({1, 2, 1, 1, 5, 6}, true));
}

BOOST_AUTO_TEST_CASE(isFreeVectorValid) {
BOOST_CHECK(!Acts::isFreeVectorValid({1, 2, 3, 4, 5, 6, 7, 8}));
BOOST_CHECK(Acts::isFreeVectorValid({1, 2, 3, 4, 1, 0, 0, 8}));
}

BOOST_AUTO_TEST_CASE(normalizeBoundParameters) {
CHECK_CLOSE_OR_SMALL(Acts::normalizeBoundParameters({1, 2, 3, 4, 5, 6}),
Acts::BoundVector(1, 2, -0.141593, 2.28319, 5, 6), 1e-3,
Expand Down
Loading

0 comments on commit ab9267b

Please sign in to comment.