Skip to content

Commit

Permalink
Merge branch 'main' into patch-6
Browse files Browse the repository at this point in the history
  • Loading branch information
AJPfleger authored Oct 30, 2024
2 parents e79eb08 + 0b8e768 commit dad4990
Show file tree
Hide file tree
Showing 75 changed files with 1,164 additions and 1,131 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ env:
CCACHE_MAXSIZE: 1.25G
CCACHE_KEY_SUFFIX: r2
ACTS_LOG_FAILURE_THRESHOLD: WARNING
DEPENDENCY_URL: https://acts.web.cern.ch/ACTS/ci/ubuntu-24.04/deps.v4.tar.zst
DEPENDENCY_URL: https://acts.web.cern.ch/ACTS/ci/ubuntu-24.04/deps.v5.tar.zst

# NOTE this only builds core unittests to reduce the output size. if we
# found a way to have Github actions not fail regularly with this job
Expand Down
24 changes: 19 additions & 5 deletions .github/workflows/builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ env:
CCACHE_DIR: ${{ github.workspace }}/ccache
CCACHE_MAXSIZE: 500M
CCACHE_KEY_SUFFIX: r2
DEPENDENCY_TAG: v5

jobs:
linux_ubuntu:
Expand All @@ -28,14 +29,16 @@ jobs:
env:
INSTALL_DIR: ${{ github.workspace }}/install
ACTS_LOG_FAILURE_THRESHOLD: WARNING
DEPENDENCY_URL: https://acts.web.cern.ch/ACTS/ci/ubuntu-24.04/deps.v4.tar.zst

steps:
- uses: actions/checkout@v4
with:
submodules: true
lfs: true

- name: Set dependencies URL
run: echo "DEPENDENCY_URL=https://acts.web.cern.ch/ACTS/ci/ubuntu-24.04/deps.${DEPENDENCY_TAG}.tar.zst" >> $GITHUB_ENV

- name: Install dependencies
run: CI/dependencies.sh

Expand Down Expand Up @@ -114,14 +117,16 @@ jobs:
needs: [linux_ubuntu]
env:
ACTS_SEQUENCER_DISABLE_FPEMON: true
DEPENDENCY_URL: https://acts.web.cern.ch/ACTS/ci/ubuntu-24.04/deps.v4.tar.zst

steps:
- uses: actions/checkout@v4
with:
submodules: true
lfs: true

- name: Set dependencies URL
run: echo "DEPENDENCY_URL=https://acts.web.cern.ch/ACTS/ci/ubuntu-24.04/deps.${DEPENDENCY_TAG}.tar.zst" >> $GITHUB_ENV

- name: Install dependencies
run: CI/dependencies.sh

Expand Down Expand Up @@ -152,7 +157,6 @@ jobs:
needs: [linux_ubuntu]
env:
ACTS_SEQUENCER_DISABLE_FPEMON: true
DEPENDENCY_URL: https://acts.web.cern.ch/ACTS/ci/ubuntu-24.04/deps.v4.tar.zst

steps:
- uses: actions/checkout@v4
Expand All @@ -161,6 +165,10 @@ jobs:
lfs: true

- run: apt-get update && apt-get install -y time

- name: Set dependencies URL
run: echo "DEPENDENCY_URL=https://acts.web.cern.ch/ACTS/ci/ubuntu-24.04/deps.${DEPENDENCY_TAG}.tar.zst" >> $GITHUB_ENV

- name: Install dependencies
run: CI/dependencies.sh

Expand Down Expand Up @@ -246,13 +254,16 @@ jobs:
env:
INSTALL_DIR: ${{ github.workspace }}/install
ACTS_LOG_FAILURE_THRESHOLD: WARNING
DEPENDENCY_URL: https://acts.web.cern.ch/ACTS/ci/ubuntu-22.04/deps.v4.tar.zst

steps:
- uses: actions/checkout@v4
with:
submodules: true
lfs: true

- name: Set dependencies URL
run: echo "DEPENDENCY_URL=https://acts.web.cern.ch/ACTS/ci/ubuntu-22.04/deps.${DEPENDENCY_TAG}.tar.zst" >> $GITHUB_ENV

- name: Install dependencies
run: CI/dependencies.sh

Expand Down Expand Up @@ -320,7 +331,7 @@ jobs:
env:
INSTALL_DIR: ${{ github.workspace }}/install_acts
ACTS_LOG_FAILURE_THRESHOLD: WARNING
DEPENDENCY_URL: https://acts.web.cern.ch/ACTS/ci/macos-14/deps.v4.tar.zst

steps:
- uses: actions/checkout@v4
with:
Expand All @@ -330,6 +341,9 @@ jobs:
- name: Print architecture
run: uname -p

- name: Set dependencies URL
run: echo "DEPENDENCY_URL=https://acts.web.cern.ch/ACTS/ci/macos-14/deps.${DEPENDENCY_TAG}.tar.zst" >> $GITHUB_ENV

- name: Install dependencies
run: >
brew install cmake ninja ccache xerces-c
Expand Down
2 changes: 1 addition & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ variables:
CCACHE_KEY_SUFFIX: r2
CTEST_OUTPUT_ON_FAILURE: 1

DEPENDENCY_TAG: v4
DEPENDENCY_TAG: v5

clang_tidy:
stage: build
Expand Down
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
3 changes: 2 additions & 1 deletion Core/include/Acts/EventData/Seed.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
namespace Acts {

template <typename external_spacepoint_t, std::size_t N = 3ul>
requires(N >= 3ul)
class Seed {
static_assert(N >= 3ul);

public:
using value_type = external_spacepoint_t;
static constexpr std::size_t DIM = N;
Expand Down
6 changes: 0 additions & 6 deletions Core/include/Acts/EventData/Seed.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -9,40 +9,34 @@
namespace Acts {

template <typename external_spacepoint_t, std::size_t N>
requires(N >= 3ul)
template <typename... args_t>
requires(sizeof...(args_t) == N) &&
(std::same_as<external_spacepoint_t, args_t> && ...)
Seed<external_spacepoint_t, N>::Seed(const args_t&... points)
: m_spacepoints({&points...}) {}

template <typename external_spacepoint_t, std::size_t N>
requires(N >= 3ul)
void Seed<external_spacepoint_t, N>::setVertexZ(float vertex) {
m_vertexZ = vertex;
}

template <typename external_spacepoint_t, std::size_t N>
requires(N >= 3ul)
void Seed<external_spacepoint_t, N>::setQuality(float seedQuality) {
m_seedQuality = seedQuality;
}

template <typename external_spacepoint_t, std::size_t N>
requires(N >= 3ul)
const std::array<const external_spacepoint_t*, N>&
Seed<external_spacepoint_t, N>::sp() const {
return m_spacepoints;
}

template <typename external_spacepoint_t, std::size_t N>
requires(N >= 3ul)
float Seed<external_spacepoint_t, N>::z() const {
return m_vertexZ;
}

template <typename external_spacepoint_t, std::size_t N>
requires(N >= 3ul)
float Seed<external_spacepoint_t, N>::seedQuality() const {
return m_seedQuality;
}
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
Loading

0 comments on commit dad4990

Please sign in to comment.