From 26273bcb6dc4979c9f484cc26550cd416932bd0d Mon Sep 17 00:00:00 2001 From: Stephen Nicholas Swatman Date: Fri, 20 Dec 2024 09:58:20 +0100 Subject: [PATCH] refactor: Add new concepts for Eigen types (#3966) 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. ## 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. --- .../Acts/EventData/TrackStateProxy.hpp | 12 ++-- .../Acts/EventData/VectorMultiTrajectory.hpp | 13 ++--- Core/include/Acts/Utilities/EigenConcepts.hpp | 56 +++++++++++++++++++ 3 files changed, 68 insertions(+), 13 deletions(-) create mode 100644 Core/include/Acts/Utilities/EigenConcepts.hpp diff --git a/Core/include/Acts/EventData/TrackStateProxy.hpp b/Core/include/Acts/EventData/TrackStateProxy.hpp index 4338c434f77..3d91547c725 100644 --- a/Core/include/Acts/EventData/TrackStateProxy.hpp +++ b/Core/include/Acts/EventData/TrackStateProxy.hpp @@ -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" @@ -782,12 +783,11 @@ class TrackStateProxy { template void allocateCalibrated(const Eigen::DenseBase& val, const Eigen::DenseBase& cov) - requires(Eigen::PlainObjectBase::RowsAtCompileTime > 0 && - Eigen::PlainObjectBase::RowsAtCompileTime <= eBoundSize && - Eigen::PlainObjectBase::RowsAtCompileTime == - Eigen::PlainObjectBase::RowsAtCompileTime && - Eigen::PlainObjectBase::RowsAtCompileTime == - Eigen::PlainObjectBase::ColsAtCompileTime) + requires(Concepts::eigen_base_is_fixed_size && + Concepts::eigen_bases_have_same_num_rows && + Concepts::eigen_base_is_square && + Eigen::PlainObjectBase::RowsAtCompileTime <= + static_cast>(eBoundSize)) { m_traj->template allocateCalibrated< Eigen::PlainObjectBase::RowsAtCompileTime>(m_istate, val, cov); diff --git a/Core/include/Acts/EventData/VectorMultiTrajectory.hpp b/Core/include/Acts/EventData/VectorMultiTrajectory.hpp index e13498cea2c..8f416dd6969 100644 --- a/Core/include/Acts/EventData/VectorMultiTrajectory.hpp +++ b/Core/include/Acts/EventData/VectorMultiTrajectory.hpp @@ -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" @@ -495,13 +496,11 @@ class VectorMultiTrajectory final void allocateCalibrated_impl(IndexType istate, const Eigen::DenseBase& val, const Eigen::DenseBase& cov) - - requires(Eigen::PlainObjectBase::RowsAtCompileTime > 0 && - Eigen::PlainObjectBase::RowsAtCompileTime <= eBoundSize && - Eigen::PlainObjectBase::RowsAtCompileTime == - Eigen::PlainObjectBase::RowsAtCompileTime && - Eigen::PlainObjectBase::RowsAtCompileTime == - Eigen::PlainObjectBase::ColsAtCompileTime) + requires(Concepts::eigen_base_is_fixed_size && + Concepts::eigen_bases_have_same_num_rows && + Concepts::eigen_base_is_square && + Eigen::PlainObjectBase::RowsAtCompileTime <= + static_cast>(eBoundSize)) { constexpr std::size_t measdim = val_t::RowsAtCompileTime; diff --git a/Core/include/Acts/Utilities/EigenConcepts.hpp b/Core/include/Acts/Utilities/EigenConcepts.hpp new file mode 100644 index 00000000000..4151dd6e237 --- /dev/null +++ b/Core/include/Acts/Utilities/EigenConcepts.hpp @@ -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 + +namespace Acts::Concepts { +/// @brief Concept that is true iff T is a valid Eigen dense base. +template +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 +concept eigen_base_is_fixed_size = + is_eigen_base && Eigen::PlainObjectBase::RowsAtCompileTime > 0 && + Eigen::PlainObjectBase::ColsAtCompileTime > 0; + +/// @brief Concept that is true iff T is a valid Eigen dense base with fixed, +/// square size. +template +concept eigen_base_is_square = eigen_base_is_fixed_size && + Eigen::PlainObjectBase::RowsAtCompileTime == + Eigen::PlainObjectBase::ColsAtCompileTime; + +/// @brief Concept that is true iff T1 and T2 have the same, known at compile +/// time, number of rows. +template +concept eigen_bases_have_same_num_rows = + eigen_base_is_fixed_size && eigen_base_is_fixed_size && + static_cast(Eigen::PlainObjectBase::RowsAtCompileTime) == + static_cast(Eigen::PlainObjectBase::RowsAtCompileTime); + +/// @brief Concept that is true iff T1 and T2 have the same, known at compile +/// time, number of columns. +template +concept eigen_bases_have_same_num_cols = + eigen_base_is_fixed_size && eigen_base_is_fixed_size && + static_cast(Eigen::PlainObjectBase::ColsAtCompileTime) == + static_cast(Eigen::PlainObjectBase::ColsAtCompileTime); + +/// @brief Concept that is true iff T1 and T2 have the same, known at compile +/// time, size. +template +concept eigen_bases_have_same_size = eigen_bases_have_same_num_rows && + eigen_bases_have_same_num_cols; +} // namespace Acts::Concepts