Skip to content

Commit

Permalink
refactor: Add new concepts for Eigen types (#3966)
Browse files Browse the repository at this point in the history
This commit adds several new concepts for dealing with Eigen base types. The motivation for this is superficially to simplify some of the requirements in the track state proxy and the multi-trajectory, but it also allows me to more easily deal with some clang compiler warnings seen in #3949, as clang won't let me compare `A::ColsAtCompileTime` and `B::ColsAtCompileTime` if `A` and `B` are different types; this allows me to hide the conversion to integers inside a concept.

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

- **New Features**
	- Introduced new concepts for Eigen matrix types to enhance type safety and clarity in the `TrackStateProxy` and `VectorMultiTrajectory` classes.
	- Updated method signatures in both classes to utilize these concepts for improved parameter validation.

- **Bug Fixes**
	- Enhanced error handling logic in the `allocateCalibrated_impl` method to ensure consistent exception messaging.

- **Documentation**
	- Added a new header file defining several concepts related to Eigen dense matrix types, improving code expressiveness and type-checking capabilities.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
stephenswat authored Dec 20, 2024
1 parent 87439ad commit 26273bc
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 13 deletions.
12 changes: 6 additions & 6 deletions Core/include/Acts/EventData/TrackStateProxy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "Acts/EventData/TrackStateType.hpp"
#include "Acts/EventData/Types.hpp"
#include "Acts/Surfaces/Surface.hpp"
#include "Acts/Utilities/EigenConcepts.hpp"
#include "Acts/Utilities/HashedString.hpp"
#include "Acts/Utilities/Helpers.hpp"

Expand Down Expand Up @@ -782,12 +783,11 @@ class TrackStateProxy {
template <typename val_t, typename cov_t>
void allocateCalibrated(const Eigen::DenseBase<val_t>& val,
const Eigen::DenseBase<cov_t>& cov)
requires(Eigen::PlainObjectBase<val_t>::RowsAtCompileTime > 0 &&
Eigen::PlainObjectBase<val_t>::RowsAtCompileTime <= eBoundSize &&
Eigen::PlainObjectBase<val_t>::RowsAtCompileTime ==
Eigen::PlainObjectBase<cov_t>::RowsAtCompileTime &&
Eigen::PlainObjectBase<cov_t>::RowsAtCompileTime ==
Eigen::PlainObjectBase<cov_t>::ColsAtCompileTime)
requires(Concepts::eigen_base_is_fixed_size<val_t> &&
Concepts::eigen_bases_have_same_num_rows<val_t, cov_t> &&
Concepts::eigen_base_is_square<cov_t> &&
Eigen::PlainObjectBase<val_t>::RowsAtCompileTime <=
static_cast<std::underlying_type_t<BoundIndices>>(eBoundSize))
{
m_traj->template allocateCalibrated<
Eigen::PlainObjectBase<val_t>::RowsAtCompileTime>(m_istate, val, cov);
Expand Down
13 changes: 6 additions & 7 deletions Core/include/Acts/EventData/VectorMultiTrajectory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "Acts/EventData/Types.hpp"
#include "Acts/EventData/detail/DynamicColumn.hpp"
#include "Acts/EventData/detail/DynamicKeyIterator.hpp"
#include "Acts/Utilities/EigenConcepts.hpp"
#include "Acts/Utilities/HashedString.hpp"
#include "Acts/Utilities/Helpers.hpp"
#include "Acts/Utilities/ThrowAssert.hpp"
Expand Down Expand Up @@ -495,13 +496,11 @@ class VectorMultiTrajectory final
void allocateCalibrated_impl(IndexType istate,
const Eigen::DenseBase<val_t>& val,
const Eigen::DenseBase<cov_t>& cov)

requires(Eigen::PlainObjectBase<val_t>::RowsAtCompileTime > 0 &&
Eigen::PlainObjectBase<val_t>::RowsAtCompileTime <= eBoundSize &&
Eigen::PlainObjectBase<val_t>::RowsAtCompileTime ==
Eigen::PlainObjectBase<cov_t>::RowsAtCompileTime &&
Eigen::PlainObjectBase<cov_t>::RowsAtCompileTime ==
Eigen::PlainObjectBase<cov_t>::ColsAtCompileTime)
requires(Concepts::eigen_base_is_fixed_size<val_t> &&
Concepts::eigen_bases_have_same_num_rows<val_t, cov_t> &&
Concepts::eigen_base_is_square<cov_t> &&
Eigen::PlainObjectBase<val_t>::RowsAtCompileTime <=
static_cast<std::underlying_type_t<BoundIndices>>(eBoundSize))
{
constexpr std::size_t measdim = val_t::RowsAtCompileTime;

Expand Down
56 changes: 56 additions & 0 deletions Core/include/Acts/Utilities/EigenConcepts.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// 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/.

#pragma once

#include <Eigen/Dense>

namespace Acts::Concepts {
/// @brief Concept that is true iff T is a valid Eigen dense base.
template <typename T>
concept is_eigen_base = requires {
{ T::RowsAtCompileTime };
{ T::ColsAtCompileTime };
};

/// @brief Concept that is true iff T is a valid Eigen dense base with fixed
/// size.
template <typename T>
concept eigen_base_is_fixed_size =
is_eigen_base<T> && Eigen::PlainObjectBase<T>::RowsAtCompileTime > 0 &&
Eigen::PlainObjectBase<T>::ColsAtCompileTime > 0;

/// @brief Concept that is true iff T is a valid Eigen dense base with fixed,
/// square size.
template <typename T>
concept eigen_base_is_square = eigen_base_is_fixed_size<T> &&
Eigen::PlainObjectBase<T>::RowsAtCompileTime ==
Eigen::PlainObjectBase<T>::ColsAtCompileTime;

/// @brief Concept that is true iff T1 and T2 have the same, known at compile
/// time, number of rows.
template <typename T1, typename T2>
concept eigen_bases_have_same_num_rows =
eigen_base_is_fixed_size<T1> && eigen_base_is_fixed_size<T2> &&
static_cast<std::size_t>(Eigen::PlainObjectBase<T1>::RowsAtCompileTime) ==
static_cast<std::size_t>(Eigen::PlainObjectBase<T2>::RowsAtCompileTime);

/// @brief Concept that is true iff T1 and T2 have the same, known at compile
/// time, number of columns.
template <typename T1, typename T2>
concept eigen_bases_have_same_num_cols =
eigen_base_is_fixed_size<T1> && eigen_base_is_fixed_size<T2> &&
static_cast<std::size_t>(Eigen::PlainObjectBase<T1>::ColsAtCompileTime) ==
static_cast<std::size_t>(Eigen::PlainObjectBase<T2>::ColsAtCompileTime);

/// @brief Concept that is true iff T1 and T2 have the same, known at compile
/// time, size.
template <typename T1, typename T2>
concept eigen_bases_have_same_size = eigen_bases_have_same_num_rows<T1, T2> &&
eigen_bases_have_same_num_cols<T1, T2>;
} // namespace Acts::Concepts

0 comments on commit 26273bc

Please sign in to comment.