From 74244eb053e501a65e2f16c03fc74bae91f864d3 Mon Sep 17 00:00:00 2001 From: Carlo Varni <75478407+CarloVarni@users.noreply.github.com> Date: Fri, 4 Oct 2024 18:22:01 +0200 Subject: [PATCH 01/18] feat!: Add a radius bin to the grid (#3662) This adds a radius bin to the grid. The binning can be provided by the user with a vector of values. If none is provided, the range `rMin -> rMax` will be used. That means that now the grids is 3D : `(phi, z, radius)` By default `rMin = 0_mm`, while `rMax = 320_mm` --- .../detail/CylindricalSpacePointGrid.hpp | 20 ++++--- .../detail/CylindricalSpacePointGrid.ipp | 54 ++++++++----------- .../TrackFinding/SeedingAlgorithm.hpp | 4 +- .../TrackFinding/SeedingAlgorithmHashing.hpp | 4 +- .../TrackFinding/src/SeedingAlgorithm.cpp | 10 ++-- .../src/SeedingAlgorithmHashing.cpp | 10 ++-- .../UnitTests/Core/Seeding/SeedFinderTest.cpp | 9 ++-- 7 files changed, 53 insertions(+), 58 deletions(-) diff --git a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp index 3acdc5a4c05..42d60158dd0 100644 --- a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp +++ b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp @@ -22,6 +22,7 @@ template using CylindricalSpacePointGrid = Acts::Grid< std::vector, Acts::Axis, + Acts::Axis, Acts::Axis>; /// Cylindrical Binned Group @@ -35,22 +36,25 @@ using CylindricalBinnedGroupIterator = Acts::BinnedGroupIterator< struct CylindricalSpacePointGridConfig { // minimum pT to be found by seedFinder - float minPt = 0; + float minPt = 0 * Acts::UnitConstants::MeV; // maximum extension of sensitive detector layer relevant for seeding as // distance from x=y=0 (i.e. in r) - float rMax = 0; + float rMax = 320 * Acts::UnitConstants::mm; + // maximum extension of sensitive detector layer relevant for seeding as + // distance from x=y=0 (i.e. in r) + float rMin = 0 * Acts::UnitConstants::mm; // maximum extension of sensitive detector layer relevant for seeding in // positive direction in z - float zMax = 0; + float zMax = 0 * Acts::UnitConstants::mm; // maximum extension of sensitive detector layer relevant for seeding in // negative direction in z - float zMin = 0; + float zMin = 0 * Acts::UnitConstants::mm; // maximum distance in r from middle space point to bottom or top spacepoint - float deltaRMax = 0; + float deltaRMax = 0 * Acts::UnitConstants::mm; // maximum forward direction expressed as cot(theta) float cotThetaMax = 0; // maximum impact parameter in mm - float impactMax = 0; + float impactMax = 0 * Acts::UnitConstants::mm; // minimum phi value for phiAxis construction float phiMin = -M_PI; // maximum phi value for phiAxis construction @@ -65,7 +69,8 @@ struct CylindricalSpacePointGridConfig { // maximum number of phi bins int maxPhiBins = 10000; // enable non equidistant binning in z - std::vector zBinEdges; + std::vector zBinEdges{}; + std::vector rBinEdges{}; bool isInInternalUnits = false; CylindricalSpacePointGridConfig toInternalUnits() const { if (isInInternalUnits) { @@ -77,6 +82,7 @@ struct CylindricalSpacePointGridConfig { CylindricalSpacePointGridConfig config = *this; config.isInInternalUnits = true; config.minPt /= 1_MeV; + config.rMin /= 1_mm; config.rMax /= 1_mm; config.zMax /= 1_mm; config.zMin /= 1_mm; diff --git a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp index a80d3105e82..5d8c886f980 100644 --- a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp +++ b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp @@ -103,7 +103,7 @@ Acts::CylindricalSpacePointGridCreator::createGrid( config.phiMin, config.phiMax, phiBins); // vector that will store the edges of the bins of z - std::vector zValues; + std::vector zValues{}; // If zBinEdges is not defined, calculate the edges as zMin + bin * zBinSize if (config.zBinEdges.empty()) { @@ -130,9 +130,19 @@ Acts::CylindricalSpacePointGridCreator::createGrid( } } + std::vector rValues{}; + rValues.reserve(std::max(2ul, config.rBinEdges.size())); + if (config.rBinEdges.empty()) { + rValues = {config.rMin, config.rMax}; + } else { + rValues.insert(rValues.end(), config.rBinEdges.begin(), + config.rBinEdges.end()); + } + Axis zAxis(std::move(zValues)); + Axis rAxis(std::move(rValues)); return Acts::CylindricalSpacePointGrid( - std::make_tuple(std::move(phiAxis), std::move(zAxis))); + std::make_tuple(std::move(phiAxis), std::move(zAxis), std::move(rAxis))); } template ( - (config.rMax + options.beamPos.norm()) / config.binSizeR); + // Space points are assumed to be ALREADY CORRECTED for beamspot position + // phi, z and r space point selection comes naturally from the + // grid axis definition. No need to explicitly cut on those values. + // If a space point is outside the validity range of these quantities + // it goes in an over- or under-flow bin. + // Additional cuts can be applied by customizing the space point selector + // in the config object. // keep track of changed bins while sorting std::vector usedBinIndex(grid.size(), false); @@ -172,37 +182,15 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid( for (external_spacepoint_iterator_t it = spBegin; it != spEnd; it++, ++counter) { const external_spacepoint_t& sp = *it; - float spX = sp.x(); - float spY = sp.y(); - float spZ = sp.z(); // remove SPs according to experiment specific cuts if (!config.spacePointSelector(sp)) { continue; } - // remove SPs outside z and phi region - if (spZ > config.zMax || spZ < config.zMin) { - continue; - } - - float spPhi = std::atan2(spY, spX); - if (spPhi > config.phiMax || spPhi < config.phiMin) { - continue; - } - - // calculate r-Bin index and protect against overflow (underflow not - // possible) - std::size_t rIndex = - static_cast(sp.radius() / config.binSizeR); - // if index out of bounds, the SP is outside the region of interest - if (rIndex >= numRBins) { - continue; - } - // fill rbins into grid - std::size_t globIndex = - grid.globalBinFromPosition(Acts::Vector2{sp.phi(), sp.z()}); + std::size_t globIndex = grid.globalBinFromPosition( + Acts::Vector3{sp.phi(), sp.z(), sp.radius()}); auto& rbin = grid.at(globIndex); rbin.push_back(&sp); diff --git a/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/SeedingAlgorithm.hpp b/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/SeedingAlgorithm.hpp index 61042cd5d09..73bb81d5d7f 100644 --- a/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/SeedingAlgorithm.hpp +++ b/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/SeedingAlgorithm.hpp @@ -100,8 +100,8 @@ class SeedingAlgorithm final : public IAlgorithm { Acts::SeedFinder> m_seedFinder; - std::unique_ptr> m_bottomBinFinder; - std::unique_ptr> m_topBinFinder; + std::unique_ptr> m_bottomBinFinder{nullptr}; + std::unique_ptr> m_topBinFinder{nullptr}; Config m_cfg; diff --git a/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/SeedingAlgorithmHashing.hpp b/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/SeedingAlgorithmHashing.hpp index 194ef4944a1..78a0ef2ae89 100644 --- a/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/SeedingAlgorithmHashing.hpp +++ b/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/SeedingAlgorithmHashing.hpp @@ -135,8 +135,8 @@ class SeedingAlgorithmHashing final : public IAlgorithm { Acts::SeedFinder> m_seedFinder; - std::unique_ptr> m_bottomBinFinder; - std::unique_ptr> m_topBinFinder; + std::unique_ptr> m_bottomBinFinder{nullptr}; + std::unique_ptr> m_topBinFinder{nullptr}; Config m_cfg; diff --git a/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp b/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp index b60acc13e85..14b89d848ca 100644 --- a/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp +++ b/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp @@ -190,10 +190,10 @@ ActsExamples::SeedingAlgorithm::SeedingAlgorithm( ActsExamples::SpacePointContainer>, Acts::detail::RefHolder>::SpacePointProxyType; - m_bottomBinFinder = std::make_unique>( - m_cfg.numPhiNeighbors, cfg.zBinNeighborsBottom); - m_topBinFinder = std::make_unique>( - m_cfg.numPhiNeighbors, m_cfg.zBinNeighborsTop); + m_bottomBinFinder = std::make_unique>( + m_cfg.numPhiNeighbors, cfg.zBinNeighborsBottom, 0); + m_topBinFinder = std::make_unique>( + m_cfg.numPhiNeighbors, m_cfg.zBinNeighborsTop, 0); m_cfg.seedFinderConfig.seedFilter = std::make_unique>( @@ -265,7 +265,7 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithm::execute( maxRange = std::max(lastEl->radius(), maxRange); } - std::array, 2ul> navigation; + std::array, 3ul> navigation; navigation[1ul] = m_cfg.seedFinderConfig.zBinsCustomLooping; auto spacePointsGrouping = Acts::CylindricalBinnedGroup( diff --git a/Examples/Algorithms/TrackFinding/src/SeedingAlgorithmHashing.cpp b/Examples/Algorithms/TrackFinding/src/SeedingAlgorithmHashing.cpp index 84011207f19..c722a9e5a2b 100644 --- a/Examples/Algorithms/TrackFinding/src/SeedingAlgorithmHashing.cpp +++ b/Examples/Algorithms/TrackFinding/src/SeedingAlgorithmHashing.cpp @@ -174,10 +174,10 @@ ActsExamples::SeedingAlgorithmHashing::SeedingAlgorithmHashing( m_cfg.seedFinderConfig.experimentCuts.connect(); } - m_bottomBinFinder = std::make_unique>( - m_cfg.numPhiNeighbors, m_cfg.zBinNeighborsBottom); - m_topBinFinder = std::make_unique>( - m_cfg.numPhiNeighbors, m_cfg.zBinNeighborsTop); + m_bottomBinFinder = std::make_unique>( + m_cfg.numPhiNeighbors, m_cfg.zBinNeighborsBottom, 0); + m_topBinFinder = std::make_unique>( + m_cfg.numPhiNeighbors, m_cfg.zBinNeighborsTop, 0); m_cfg.seedFinderConfig.seedFilter = std::make_unique>( @@ -291,7 +291,7 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithmHashing::execute( maxRange = std::max(lastEl->radius(), maxRange); } - std::array, 2ul> navigation; + std::array, 3ul> navigation; navigation[1ul] = m_cfg.seedFinderConfig.zBinsCustomLooping; // groups spacepoints diff --git a/Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp b/Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp index 08bec8a4d92..9d89c1c96c6 100644 --- a/Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp +++ b/Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp @@ -145,6 +145,7 @@ int main(int argc, char** argv) { Acts::SeedFinderConfig config; // silicon detector max config.rMax = 160._mm; + config.rMin = 0._mm; config.deltaRMin = 5._mm; config.deltaRMax = 160._mm; config.deltaRMinTopSP = config.deltaRMin; @@ -178,10 +179,10 @@ int main(int argc, char** argv) { std::vector> zBinNeighborsTop; std::vector> zBinNeighborsBottom; - auto bottomBinFinder = std::make_unique>( - Acts::GridBinFinder<2ul>(numPhiNeighbors, zBinNeighborsBottom)); - auto topBinFinder = std::make_unique>( - Acts::GridBinFinder<2ul>(numPhiNeighbors, zBinNeighborsTop)); + auto bottomBinFinder = std::make_unique>( + numPhiNeighbors, zBinNeighborsBottom, 0); + auto topBinFinder = std::make_unique>( + numPhiNeighbors, zBinNeighborsTop, 0); Acts::SeedFilterConfig sfconf; Acts::ATLASCuts atlasCuts = Acts::ATLASCuts(); From 6cea6b2ddf6fb6c47d2fc9495463643fbbee7890 Mon Sep 17 00:00:00 2001 From: Carlo Varni <75478407+CarloVarni@users.noreply.github.com> Date: Fri, 4 Oct 2024 19:32:11 +0200 Subject: [PATCH 02/18] feat: Add support for Timed Clusterization (#3654) This add possibility of doing clusterization using as well time information --- .../Acts/Clusterization/Clusterization.hpp | 50 +++- .../Acts/Clusterization/Clusterization.ipp | 100 ++----- .../Clusterization/TimedClusterization.hpp | 38 +++ .../Clusterization/TimedClusterization.ipp | 36 +++ .../Core/Clusterization/CMakeLists.txt | 1 + .../TimedClusterizationTests.cpp | 260 ++++++++++++++++++ 6 files changed, 401 insertions(+), 84 deletions(-) create mode 100644 Core/include/Acts/Clusterization/TimedClusterization.hpp create mode 100644 Core/include/Acts/Clusterization/TimedClusterization.ipp create mode 100644 Tests/UnitTests/Core/Clusterization/TimedClusterizationTests.cpp diff --git a/Core/include/Acts/Clusterization/Clusterization.hpp b/Core/include/Acts/Clusterization/Clusterization.hpp index 3ebfd8d562d..a7b4d853b20 100644 --- a/Core/include/Acts/Clusterization/Clusterization.hpp +++ b/Core/include/Acts/Clusterization/Clusterization.hpp @@ -13,6 +13,26 @@ namespace Acts::Ccl { +template +concept HasRetrievableColumnInfo = requires(Cell cell) { + { getCellColumn(cell) } -> std::same_as; +}; + +template +concept HasRetrievableRowInfo = requires(Cell cell) { + { getCellRow(cell) } -> std::same_as; +}; + +template +concept HasRetrievableLabelInfo = requires(Cell cell) { + { getCellLabel(cell) } -> std::same_as; +}; + +template +concept CanAcceptCell = requires(Cell cell, Cluster cluster) { + { clusterAddCell(cluster, cell) } -> std::same_as; +}; + using Label = int; constexpr Label NO_LABEL = 0; @@ -28,17 +48,21 @@ enum class ConnectResult { // Default connection type for 2-D grids: 4- or 8-cell connectivity template + requires(Acts::Ccl::HasRetrievableColumnInfo && + Acts::Ccl::HasRetrievableRowInfo) struct Connect2D { - bool conn8; - Connect2D() : conn8{true} {} + bool conn8{true}; + Connect2D() = default; explicit Connect2D(bool commonCorner) : conn8{commonCorner} {} - ConnectResult operator()(const Cell& ref, const Cell& iter) const; + virtual ConnectResult operator()(const Cell& ref, const Cell& iter) const; + virtual ~Connect2D() = default; }; // Default connection type for 1-D grids: 2-cell connectivity -template +template struct Connect1D { - ConnectResult operator()(const Cell& ref, const Cell& iter) const; + virtual ConnectResult operator()(const Cell& ref, const Cell& iter) const; + virtual ~Connect1D() = default; }; // Default connection type based on GridDim @@ -49,13 +73,16 @@ struct DefaultConnect { }; template -struct DefaultConnect : public Connect2D { - explicit DefaultConnect(bool commonCorner) : Connect2D(commonCorner) {} - DefaultConnect() : DefaultConnect(true) {} +struct DefaultConnect : public Connect1D { + ~DefaultConnect() override = default; }; template -struct DefaultConnect : public Connect1D {}; +struct DefaultConnect : public Connect2D { + explicit DefaultConnect(bool commonCorner) : Connect2D(commonCorner) {} + DefaultConnect() = default; + ~DefaultConnect() override = default; +}; /// @brief labelClusters /// @@ -70,6 +97,8 @@ struct DefaultConnect : public Connect1D {}; template > + requires( + Acts::Ccl::HasRetrievableLabelInfo) void labelClusters(CellCollection& cells, Connect connect = Connect()); /// @brief mergeClusters @@ -82,6 +111,9 @@ void labelClusters(CellCollection& cells, Connect connect = Connect()); /// @return nothing template + requires(GridDim == 1 || GridDim == 2) && + Acts::Ccl::HasRetrievableLabelInfo< + typename CellCollection::value_type> ClusterCollection mergeClusters(CellCollection& /*cells*/); /// @brief createClusters diff --git a/Core/include/Acts/Clusterization/Clusterization.ipp b/Core/include/Acts/Clusterization/Clusterization.ipp index 0aa79b53981..90a69930ce1 100644 --- a/Core/include/Acts/Clusterization/Clusterization.ipp +++ b/Core/include/Acts/Clusterization/Clusterization.ipp @@ -14,60 +14,6 @@ namespace Acts::Ccl::internal { -// Machinery for validating generic Cell/Cluster types at compile-time - -template -struct cellTypeHasRequiredFunctions : std::false_type {}; - -template -struct cellTypeHasRequiredFunctions< - T, 2, - std::void_t())), - decltype(getCellColumn(std::declval())), - decltype(getCellLabel(std::declval()))>> : std::true_type { -}; - -template -struct cellTypeHasRequiredFunctions< - T, 1, - std::void_t())), - decltype(getCellLabel(std::declval()))>> : std::true_type { -}; - -template -struct clusterTypeHasRequiredFunctions : std::false_type {}; - -template -struct clusterTypeHasRequiredFunctions< - T, U, - std::void_t(), std::declval()))>> - : std::true_type {}; - -template -constexpr void staticCheckGridDim() { - static_assert( - GridDim == 1 || GridDim == 2, - "mergeClusters is only defined for grid dimensions of 1 or 2. "); -} - -template -constexpr void staticCheckCellType() { - constexpr bool hasFns = cellTypeHasRequiredFunctions(); - static_assert(hasFns, - "Cell type should have the following functions: " - "'int getCellRow(const Cell&)', " - "'int getCellColumn(const Cell&)', " - "'Label& getCellLabel(Cell&)'"); -} - -template -constexpr void staticCheckClusterType() { - constexpr bool hasFns = clusterTypeHasRequiredFunctions(); - static_assert(hasFns, - "Cluster type should have the following function: " - "'void clusterAddCell(Cluster&, const Cell&)'"); -} - template struct Compare { static_assert(GridDim != 1 && GridDim != 2, @@ -75,25 +21,27 @@ struct Compare { }; // Comparator function object for cells, column-wise ordering -// Specialization for 2-D grid -template -struct Compare { +// Specialization for 1-D grids +template +struct Compare { bool operator()(const Cell& c0, const Cell& c1) const { - int row0 = getCellRow(c0); - int row1 = getCellRow(c1); int col0 = getCellColumn(c0); int col1 = getCellColumn(c1); - return (col0 == col1) ? row0 < row1 : col0 < col1; + return col0 < col1; } }; -// Specialization for 1-D grids +// Specialization for 2-D grid template -struct Compare { + requires(Acts::Ccl::HasRetrievableColumnInfo && + Acts::Ccl::HasRetrievableRowInfo) +struct Compare { bool operator()(const Cell& c0, const Cell& c1) const { + int row0 = getCellRow(c0); + int row1 = getCellRow(c1); int col0 = getCellColumn(c0); int col1 = getCellColumn(c1); - return col0 < col1; + return (col0 == col1) ? row0 < row1 : col0 < col1; } }; @@ -184,6 +132,10 @@ Connections getConnections(typename std::vector::iterator it, } template + requires( + Acts::Ccl::HasRetrievableLabelInfo && + Acts::Ccl::CanAcceptCell) ClusterCollection mergeClustersImpl(CellCollection& cells) { using Cluster = typename ClusterCollection::value_type; @@ -215,6 +167,8 @@ ClusterCollection mergeClustersImpl(CellCollection& cells) { namespace Acts::Ccl { template + requires(Acts::Ccl::HasRetrievableColumnInfo && + Acts::Ccl::HasRetrievableRowInfo) ConnectResult Connect2D::operator()(const Cell& ref, const Cell& iter) const { int deltaRow = std::abs(getCellRow(ref) - getCellRow(iter)); @@ -237,7 +191,7 @@ ConnectResult Connect2D::operator()(const Cell& ref, return ConnectResult::eNoConn; } -template +template ConnectResult Connect1D::operator()(const Cell& ref, const Cell& iter) const { int deltaCol = std::abs(getCellColumn(ref) - getCellColumn(iter)); @@ -267,9 +221,10 @@ void recordEquivalences(const internal::Connections seen, } template + requires( + Acts::Ccl::HasRetrievableLabelInfo) void labelClusters(CellCollection& cells, Connect connect) { using Cell = typename CellCollection::value_type; - internal::staticCheckCellType(); internal::DisjointSets ds{}; @@ -277,7 +232,8 @@ void labelClusters(CellCollection& cells, Connect connect) { std::ranges::sort(cells, internal::Compare()); // First pass: Allocate labels and record equivalences - for (auto it = cells.begin(); it != cells.end(); ++it) { + for (auto it = std::ranges::begin(cells); it != std::ranges::end(cells); + ++it) { const internal::Connections seen = internal::getConnections(it, cells, connect); if (seen.nconn == 0) { @@ -299,13 +255,11 @@ void labelClusters(CellCollection& cells, Connect connect) { template + requires(GridDim == 1 || GridDim == 2) && + Acts::Ccl::HasRetrievableLabelInfo< + typename CellCollection::value_type> ClusterCollection mergeClusters(CellCollection& cells) { using Cell = typename CellCollection::value_type; - using Cluster = typename ClusterCollection::value_type; - internal::staticCheckGridDim(); - internal::staticCheckCellType(); - internal::staticCheckClusterType(); - if constexpr (GridDim > 1) { // Sort the cells by their cluster label, only needed if more than // one spatial dimension @@ -318,10 +272,6 @@ ClusterCollection mergeClusters(CellCollection& cells) { template ClusterCollection createClusters(CellCollection& cells, Connect connect) { - using Cell = typename CellCollection::value_type; - using Cluster = typename ClusterCollection::value_type; - internal::staticCheckCellType(); - internal::staticCheckClusterType(); labelClusters(cells, connect); return mergeClusters(cells); } diff --git a/Core/include/Acts/Clusterization/TimedClusterization.hpp b/Core/include/Acts/Clusterization/TimedClusterization.hpp new file mode 100644 index 00000000000..e92ebce44e2 --- /dev/null +++ b/Core/include/Acts/Clusterization/TimedClusterization.hpp @@ -0,0 +1,38 @@ +// 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 "Acts/Clusterization/Clusterization.hpp" +#include "Acts/Definitions/Algebra.hpp" + +#include + +namespace Acts::Ccl { + +template +concept HasRetrievableTimeInfo = requires(Cell cell) { + { getCellTime(cell) } -> std::same_as; +}; + +template +struct TimedConnect : public Acts::Ccl::DefaultConnect { + Acts::ActsScalar timeTolerance{std::numeric_limits::max()}; + + TimedConnect() = default; + TimedConnect(Acts::ActsScalar time); + TimedConnect(Acts::ActsScalar time, bool commonCorner) + requires(N == 2); + ~TimedConnect() override = default; + + ConnectResult operator()(const Cell& ref, const Cell& iter) const override; +}; + +} // namespace Acts::Ccl + +#include "Acts/Clusterization/TimedClusterization.ipp" diff --git a/Core/include/Acts/Clusterization/TimedClusterization.ipp b/Core/include/Acts/Clusterization/TimedClusterization.ipp new file mode 100644 index 00000000000..0e7b3e5bda8 --- /dev/null +++ b/Core/include/Acts/Clusterization/TimedClusterization.ipp @@ -0,0 +1,36 @@ +// 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/. + +namespace Acts::Ccl { + +template +TimedConnect::TimedConnect(Acts::ActsScalar time) + : timeTolerance(time) {} + +template +TimedConnect::TimedConnect(Acts::ActsScalar time, bool commonCorner) + requires(N == 2) + : Acts::Ccl::DefaultConnect(commonCorner), timeTolerance(time) {} + +template +Acts::Ccl::ConnectResult TimedConnect::operator()( + const Cell& ref, const Cell& iter) const { + Acts::Ccl::ConnectResult spaceCompatibility = + Acts::Ccl::DefaultConnect::operator()(ref, iter); + if (spaceCompatibility != Acts::Ccl::ConnectResult::eConn) { + return spaceCompatibility; + } + + if (std::abs(getCellTime(ref) - getCellTime(iter)) < timeTolerance) { + return Acts::Ccl::ConnectResult::eConn; + } + + return Acts::Ccl::ConnectResult::eNoConn; +} + +} // namespace Acts::Ccl diff --git a/Tests/UnitTests/Core/Clusterization/CMakeLists.txt b/Tests/UnitTests/Core/Clusterization/CMakeLists.txt index 38dffefd462..a6c4844bdb9 100644 --- a/Tests/UnitTests/Core/Clusterization/CMakeLists.txt +++ b/Tests/UnitTests/Core/Clusterization/CMakeLists.txt @@ -1,2 +1,3 @@ add_unittest(Clusterization1D ClusterizationTests1D.cpp) add_unittest(Clusterization2D ClusterizationTests2D.cpp) +add_unittest(TimedClusterization TimedClusterizationTests.cpp) diff --git a/Tests/UnitTests/Core/Clusterization/TimedClusterizationTests.cpp b/Tests/UnitTests/Core/Clusterization/TimedClusterizationTests.cpp new file mode 100644 index 00000000000..ac60393d9b6 --- /dev/null +++ b/Tests/UnitTests/Core/Clusterization/TimedClusterizationTests.cpp @@ -0,0 +1,260 @@ +// 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 + +#include "Acts/Clusterization/TimedClusterization.hpp" + +namespace Acts::Test { + +// Define objects +using Identifier = std::size_t; +struct Cell { + Cell(Identifier identifier, int c, int r, double t) + : id(identifier), column(c), row(r), time(t) {} + + Identifier id{}; + int column{0}; + int row{0}; + int label{-1}; + double time{0.}; +}; + +struct Cluster { + std::vector ids{}; +}; + +using CellCollection = std::vector; +using ClusterCollection = std::vector; + +// Define functions +static inline int getCellRow(const Cell& cell) { + return cell.row; +} + +static inline int getCellColumn(const Cell& cell) { + return cell.column; +} + +static inline int& getCellLabel(Cell& cell) { + return cell.label; +} + +static inline double getCellTime(const Cell& cell) { + return cell.time; +} + +static void clusterAddCell(Cluster& cl, const Cell& cell) { + cl.ids.push_back(cell.id); +} + +BOOST_AUTO_TEST_CASE(TimedGrid_1D_withtime) { + // 1x10 matrix + /* + X X X Y O X Y Y X X + */ + // 6 + 3 cells -> 3 + 2 clusters in total + + std::vector cells; + // X + cells.emplace_back(0ul, 0, -1, 0); + cells.emplace_back(1ul, 1, -1, 0); + cells.emplace_back(2ul, 2, -1, 0); + cells.emplace_back(3ul, 5, -1, 0); + cells.emplace_back(4ul, 8, -1, 0); + cells.emplace_back(5ul, 9, -1, 0); + // Y + cells.emplace_back(6ul, 3, 0, 1); + cells.emplace_back(7ul, 6, 1, 1); + cells.emplace_back(8ul, 7, 1, 1); + + std::vector> expectedResults; + expectedResults.push_back({0ul, 1ul, 2ul}); + expectedResults.push_back({6ul}); + expectedResults.push_back({3ul}); + expectedResults.push_back({7ul, 8ul}); + expectedResults.push_back({4ul, 5ul}); + + ClusterCollection clusters = + Acts::Ccl::createClusters( + cells, Acts::Ccl::TimedConnect(0.5)); + + BOOST_CHECK_EQUAL(5ul, clusters.size()); + + for (std::size_t i(0); i < clusters.size(); ++i) { + std::vector& timedIds = clusters[i].ids; + const std::vector& expected = expectedResults[i]; + std::sort(timedIds.begin(), timedIds.end()); + BOOST_CHECK_EQUAL(timedIds.size(), expected.size()); + + for (std::size_t j(0); j < timedIds.size(); ++j) { + BOOST_CHECK_EQUAL(timedIds[j], expected[j]); + } + } +} + +BOOST_AUTO_TEST_CASE(TimedGrid_2D_notime) { + // 4x4 matrix + /* + X O O X + O O O X + X X O O + X O O X + */ + // 7 cells -> 4 clusters in total + + std::vector cells; + cells.emplace_back(0ul, 0, 0, 0); + cells.emplace_back(1ul, 3, 0, 0); + cells.emplace_back(2ul, 3, 1, 0); + cells.emplace_back(3ul, 0, 2, 0); + cells.emplace_back(4ul, 1, 2, 0); + cells.emplace_back(5ul, 0, 3, 0); + cells.emplace_back(6ul, 3, 3, 0); + + std::vector> expectedResults; + expectedResults.push_back({0ul}); + expectedResults.push_back({3ul, 4ul, 5ul}); + expectedResults.push_back({1ul, 2ul}); + expectedResults.push_back({6ul}); + + ClusterCollection clusters = + Acts::Ccl::createClusters( + cells, + Acts::Ccl::TimedConnect(std::numeric_limits::max())); + + BOOST_CHECK_EQUAL(4ul, clusters.size()); + + // Compare against default connect (only space) + ClusterCollection defaultClusters = + Acts::Ccl::createClusters( + cells, Acts::Ccl::DefaultConnect()); + + BOOST_CHECK_EQUAL(4ul, defaultClusters.size()); + BOOST_CHECK_EQUAL(defaultClusters.size(), expectedResults.size()); + + std::vector sizes{1, 3, 2, 1}; + for (std::size_t i(0); i < clusters.size(); ++i) { + std::vector& timedIds = clusters[i].ids; + std::vector& defaultIds = defaultClusters[i].ids; + const std::vector& expected = expectedResults[i]; + BOOST_CHECK_EQUAL(timedIds.size(), defaultIds.size()); + BOOST_CHECK_EQUAL(timedIds.size(), sizes[i]); + BOOST_CHECK_EQUAL(timedIds.size(), expected.size()); + + std::sort(timedIds.begin(), timedIds.end()); + std::sort(defaultIds.begin(), defaultIds.end()); + for (std::size_t j(0); j < timedIds.size(); ++j) { + BOOST_CHECK_EQUAL(timedIds[j], defaultIds[j]); + BOOST_CHECK_EQUAL(timedIds[j], expected[j]); + } + } +} + +BOOST_AUTO_TEST_CASE(TimedGrid_2D_withtime) { + // 4x4 matrix + /* + X Y O X + O Y Y X + X X Z Z + X O O X + */ + // 7 + 3 + 2 cells -> 4 + 1 + 1 clusters in total + + std::vector cells; + // X + cells.emplace_back(0ul, 0, 0, 0); + cells.emplace_back(1ul, 3, 0, 0); + cells.emplace_back(2ul, 3, 1, 0); + cells.emplace_back(3ul, 0, 2, 0); + cells.emplace_back(4ul, 1, 2, 0); + cells.emplace_back(5ul, 0, 3, 0); + cells.emplace_back(6ul, 3, 3, 0); + // Y + cells.emplace_back(7ul, 1, 0, 1); + cells.emplace_back(8ul, 1, 1, 1); + cells.emplace_back(9ul, 2, 1, 1); + // Z + cells.emplace_back(10ul, 2, 2, 2); + cells.emplace_back(11ul, 3, 2, 2); + + std::vector> expectedResults; + expectedResults.push_back({0ul}); + expectedResults.push_back({3ul, 4ul, 5ul}); + expectedResults.push_back({7ul, 8ul, 9ul}); + expectedResults.push_back({10ul, 11ul}); + expectedResults.push_back({1ul, 2ul}); + expectedResults.push_back({6ul}); + + ClusterCollection clusters = + Acts::Ccl::createClusters( + cells, Acts::Ccl::TimedConnect(0.5)); + + BOOST_CHECK_EQUAL(6ul, clusters.size()); + + std::vector sizes{1, 3, 3, 2, 2, 1}; + for (std::size_t i(0); i < clusters.size(); ++i) { + std::vector& timedIds = clusters[i].ids; + BOOST_CHECK_EQUAL(timedIds.size(), sizes[i]); + std::sort(timedIds.begin(), timedIds.end()); + + const std::vector& expected = expectedResults[i]; + BOOST_CHECK_EQUAL(timedIds.size(), expected.size()); + + for (std::size_t j(0); j < timedIds.size(); ++j) { + BOOST_CHECK_EQUAL(timedIds[j], expected[j]); + } + } +} + +BOOST_AUTO_TEST_CASE(TimedGrid_2D_noTollerance) { + // 4x4 matrix + /* + X O O X + O O O X + X X O O + X O O X + */ + // 7 cells -> 7 clusters in total + // since time requirement will never be satisfied + + std::vector cells; + cells.emplace_back(0ul, 0, 0, 0); + cells.emplace_back(1ul, 3, 0, 0); + cells.emplace_back(2ul, 3, 1, 0); + cells.emplace_back(3ul, 0, 2, 0); + cells.emplace_back(4ul, 1, 2, 0); + cells.emplace_back(5ul, 0, 3, 0); + cells.emplace_back(6ul, 3, 3, 0); + + std::vector> expectedResults; + expectedResults.push_back({0ul}); + expectedResults.push_back({3ul}); + expectedResults.push_back({5ul}); + expectedResults.push_back({4ul}); + expectedResults.push_back({1ul}); + expectedResults.push_back({2ul}); + expectedResults.push_back({6ul}); + + ClusterCollection clusters = + Acts::Ccl::createClusters( + cells, Acts::Ccl::TimedConnect(0.)); + + BOOST_CHECK_EQUAL(7ul, clusters.size()); + + for (std::size_t i(0); i < clusters.size(); ++i) { + std::vector& timedIds = clusters[i].ids; + const std::vector& expected = expectedResults[i]; + + BOOST_CHECK_EQUAL(timedIds.size(), 1); + BOOST_CHECK_EQUAL(timedIds.size(), expected.size()); + BOOST_CHECK_EQUAL(timedIds[0], expected[0]); + } +} + +} // namespace Acts::Test From 238639ba370cc6ed1723a8cf95a7b0d140f3e365 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 4 Oct 2024 20:53:42 +0200 Subject: [PATCH 03/18] refactor(geo): Add portals + surfaces to closeGeometry and visitSurfaces (#3678) Also includes some cleanup. Related to #3502 Blocked by: - https://github.com/acts-project/acts/pull/3675 --- Core/include/Acts/Geometry/TrackingVolume.hpp | 13 ++++++ Core/src/Geometry/TrackingVolume.cpp | 45 +++++++++++++++++-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/Core/include/Acts/Geometry/TrackingVolume.hpp b/Core/include/Acts/Geometry/TrackingVolume.hpp index b208eefa54b..1d1a5b6dc3c 100644 --- a/Core/include/Acts/Geometry/TrackingVolume.hpp +++ b/Core/include/Acts/Geometry/TrackingVolume.hpp @@ -15,6 +15,7 @@ #include "Acts/Geometry/GeometryIdentifier.hpp" #include "Acts/Geometry/GlueVolumesDescriptor.hpp" #include "Acts/Geometry/Layer.hpp" +#include "Acts/Geometry/Portal.hpp" #include "Acts/Geometry/TrackingVolumeVisitorConcept.hpp" #include "Acts/Geometry/Volume.hpp" #include "Acts/Material/IVolumeMaterial.hpp" @@ -181,6 +182,10 @@ class TrackingVolume : public Volume { for (const auto& bs : m_boundarySurfaces) { visitor(&(bs->surfaceRepresentation())); } + + for (const auto& portal : portals()) { + visitor(&portal.surface()); + } } // Internal structure @@ -214,6 +219,14 @@ class TrackingVolume : public Volume { volume->visitSurfaces(visitor, restrictToSensitives); } } + + for (const auto& surface : surfaces()) { + visitor(&surface); + } + + for (const auto& volume : volumes()) { + volume.visitSurfaces(visitor, restrictToSensitives); + } } /// @brief Visit all sensitive surfaces diff --git a/Core/src/Geometry/TrackingVolume.cpp b/Core/src/Geometry/TrackingVolume.cpp index 79ffdc9d180..9bcf9cc969f 100644 --- a/Core/src/Geometry/TrackingVolume.cpp +++ b/Core/src/Geometry/TrackingVolume.cpp @@ -347,6 +347,31 @@ void TrackingVolume::closeGeometry( std::unordered_map& volumeMap, std::size_t& vol, const GeometryIdentifierHook& hook, const Logger& logger) { + if (!boundarySurfaces().empty() && !portals().empty()) { + ACTS_ERROR( + "TrackingVolume::closeGeometry: Volume " + << volumeName() + << " has both boundary surfaces and portals. This is not supported."); + throw std::invalid_argument( + "Volume has both boundary surfaces and portals"); + } + + if (m_confinedVolumes && !volumes().empty()) { + ACTS_ERROR( + "TrackingVolume::closeGeometry: Volume " + << volumeName() + << " has both confined volumes and volumes. This is not supported."); + throw std::invalid_argument("Volume has both confined volumes and volumes"); + } + + if (m_confinedLayers && !surfaces().empty()) { + ACTS_ERROR( + "TrackingVolume::closeGeometry: Volume " + << volumeName() + << " has both confined layers and surfaces. This is not supported."); + throw std::invalid_argument("Volume has both confined layers and surfaces"); + } + // we can construct the volume ID from this auto volumeID = GeometryIdentifier().setVolume(++vol); // assign the Volume ID to the volume itself @@ -379,7 +404,8 @@ void TrackingVolume::closeGeometry( // get the intersection solution auto& bSurface = bSurfIter->surfaceRepresentation(); // create the boundary surface id - auto boundaryID = GeometryIdentifier(volumeID).setBoundary(++iboundary); + iboundary++; + auto boundaryID = GeometryIdentifier(volumeID).setBoundary(iboundary); // now assign to the boundary surface auto& mutableBSurface = *(const_cast(&bSurface)); mutableBSurface.assignGeometryId(boundaryID); @@ -397,7 +423,8 @@ void TrackingVolume::closeGeometry( // loop over the layers for (auto& layerPtr : m_confinedLayers->arrayObjects()) { // create the layer identification - auto layerID = GeometryIdentifier(volumeID).setLayer(++ilayer); + ilayer++; + auto layerID = GeometryIdentifier(volumeID).setLayer(ilayer); // now close the geometry auto mutableLayerPtr = std::const_pointer_cast(layerPtr); mutableLayerPtr->closeGeometry(materialDecorator, layerID, hook, @@ -428,12 +455,24 @@ void TrackingVolume::closeGeometry( GeometryIdentifier::Value iportal = 0; for (auto& portal : portals()) { - auto portalId = GeometryIdentifier(volumeID).setBoundary(++iportal); + iportal++; + auto portalId = GeometryIdentifier(volumeID).setBoundary(iportal); assert(portal.isValid() && "Invalid portal encountered during closing"); portal.surface().assignGeometryId(portalId); } + GeometryIdentifier::Value isensitive = 0; + + for (auto& surface : surfaces()) { + if (surface.associatedDetectorElement() == nullptr) { + continue; + } + isensitive++; + auto sensitiveId = GeometryIdentifier(volumeID).setSensitive(isensitive); + surface.assignGeometryId(sensitiveId); + } + for (auto& volume : volumes()) { volume.closeGeometry(materialDecorator, volumeMap, vol, hook, logger); } From c96fe3835e20aa2ff2d116ed6ac405581cbf27d5 Mon Sep 17 00:00:00 2001 From: Carlo Varni <75478407+CarloVarni@users.noreply.github.com> Date: Sun, 6 Oct 2024 22:09:13 +0200 Subject: [PATCH 04/18] refactor: use std::atan2 instead of atan2f (#3695) --- Core/include/Acts/EventData/SpacePointContainer.hpp | 2 -- Core/include/Acts/EventData/SpacePointContainer.ipp | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Core/include/Acts/EventData/SpacePointContainer.hpp b/Core/include/Acts/EventData/SpacePointContainer.hpp index 5a7be3b23f7..619a1393ecc 100644 --- a/Core/include/Acts/EventData/SpacePointContainer.hpp +++ b/Core/include/Acts/EventData/SpacePointContainer.hpp @@ -19,8 +19,6 @@ #include #include -#include - namespace Acts { struct SpacePointContainerConfig { diff --git a/Core/include/Acts/EventData/SpacePointContainer.ipp b/Core/include/Acts/EventData/SpacePointContainer.ipp index 01800be7cd5..e0c381e4a50 100644 --- a/Core/include/Acts/EventData/SpacePointContainer.ipp +++ b/Core/include/Acts/EventData/SpacePointContainer.ipp @@ -6,7 +6,7 @@ // 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 +#include namespace Acts { @@ -44,7 +44,7 @@ void SpacePointContainer::initialize() { m_data.setZ(i, external_container.z_impl(i)); m_data.setRadius( i, std::sqrt(m_data.x(i) * m_data.x(i) + m_data.y(i) * m_data.y(i))); - m_data.setPhi(i, atan2f(m_data.y(i), m_data.x(i))); + m_data.setPhi(i, std::atan2(m_data.y(i), m_data.x(i))); m_data.setVarianceR(i, external_container.varianceR_impl(i)); m_data.setVarianceZ(i, external_container.varianceZ_impl(i)); From 26a4a8ff14813b43f3f36fceebee5fc8fdabe8c9 Mon Sep 17 00:00:00 2001 From: Giacomo Alocco <40990466+galocco@users.noreply.github.com> Date: Mon, 7 Oct 2024 09:50:59 +0200 Subject: [PATCH 05/18] refactor: set convertMaterial as configurable (#3604) The default value of convertMaterial is false to allow the conversion of all materials (not only primitives). convertMaterial can now be set in the createSurface python binding. --- Examples/Python/src/Geant4Component.cpp | 4 +++- .../Acts/Plugins/Geant4/Geant4DetectorSurfaceFactory.hpp | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Examples/Python/src/Geant4Component.cpp b/Examples/Python/src/Geant4Component.cpp index 14e7b138888..ff34da94cc0 100644 --- a/Examples/Python/src/Geant4Component.cpp +++ b/Examples/Python/src/Geant4Component.cpp @@ -334,7 +334,8 @@ PYBIND11_MODULE(ActsPythonBindingsGeant4, mod) { const std::vector& sensitiveMatches, const std::vector& - passiveMatches) { + passiveMatches, + bool convertMaterial) { // Initiate the detector construction & retrieve world ActsExamples::GdmlDetectorConstruction gdmlContruction(gdmlFileName); const auto* world = gdmlContruction.Construct(); @@ -351,6 +352,7 @@ PYBIND11_MODULE(ActsPythonBindingsGeant4, mod) { Acts::Geant4DetectorSurfaceFactory::Options options; options.sensitiveSurfaceSelector = sensitiveSelectors; options.passiveSurfaceSelector = passiveSelectors; + options.convertMaterial = convertMaterial; G4Transform3D nominal; Acts::Geant4DetectorSurfaceFactory factory; diff --git a/Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4DetectorSurfaceFactory.hpp b/Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4DetectorSurfaceFactory.hpp index fae8fc20fea..d06c77412e4 100644 --- a/Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4DetectorSurfaceFactory.hpp +++ b/Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4DetectorSurfaceFactory.hpp @@ -67,7 +67,7 @@ class Geant4DetectorSurfaceFactory { /// Convert the length scale ActsScalar scaleConversion = 1.; /// Convert the material - bool convertMaterial = true; + bool convertMaterial = false; /// Converted material thickness (< 0 indicates keeping original thickness) ActsScalar convertedMaterialThickness = -1; /// A selector for sensitive surfaces From 8f0ae3cbbecafb1a295719fc25949f8e1c6d2589 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Mon, 7 Oct 2024 12:05:10 +0200 Subject: [PATCH 06/18] refactor(geo): Geometry visualization update (#3681) Blocked by: - https://github.com/acts-project/acts/pull/3675 Related to #3502 --- The most significant changes include adding visualization methods to `TrackingGeometry` and `TrackingVolume`, updating the `Surface` class, and enhancing the `IVisualization3D` interface. Additionally, the `ObjVisualization3D` class has been refactored, and new default view configurations have been defined. ### Visualization Enhancements: * [`TrackingGeometry.hpp`](diffhunk://#diff-cc497a4ee5032615db90ef065e9466279a0fde38fbd201840a32d783d3d6ec4aR148-R158): Added a `visualize` method to enable visualization of tracking geometry, including substructures. (`[Core/include/Acts/Geometry/TrackingGeometry.hppR148-R158](diffhunk://#diff-cc497a4ee5032615db90ef065e9466279a0fde38fbd201840a32d783d3d6ec4aR148-R158)`) * [`TrackingVolume.hpp`](diffhunk://#diff-835fb549fb77cdaa632a4e2131c8dc17ad4973ab3f1600a4608582b706a3f68eR326-R344): Added methods to visualize tracking volumes and manage surfaces, including `visualize`, `surfaces`, and `addSurface`. (`[[1]](diffhunk://#diff-835fb549fb77cdaa632a4e2131c8dc17ad4973ab3f1600a4608582b706a3f68eR326-R344)`, `[[2]](diffhunk://#diff-835fb549fb77cdaa632a4e2131c8dc17ad4973ab3f1600a4608582b706a3f68eR477-R487)`, `[[3]](diffhunk://#diff-835fb549fb77cdaa632a4e2131c8dc17ad4973ab3f1600a4608582b706a3f68eR550)`) ### Interface Updates: * [`IVisualization3D.hpp`](diffhunk://#diff-c56ccee98d351daf6ac595cd265d30716a594ceb74030138411595aca66fdc39R75-R80): Introduced a destructor and a new `object` method to start a new object context. (`[Core/include/Acts/Visualization/IVisualization3D.hppR75-R80](diffhunk://#diff-c56ccee98d351daf6ac595cd265d30716a594ceb74030138411595aca66fdc39R75-R80)`) * [`ObjVisualization3D.hpp`](diffhunk://#diff-fab69ace861a3e5d8fc173e8627732777a5fdd52703e9e162a14db563ea56a2aL29-R32): Refactored to use `double` as the value type and added support for object contexts. (`[[1]](diffhunk://#diff-fab69ace861a3e5d8fc173e8627732777a5fdd52703e9e162a14db563ea56a2aL29-R32)`, `[[2]](diffhunk://#diff-fab69ace861a3e5d8fc173e8627732777a5fdd52703e9e162a14db563ea56a2aR76-R102)`) ### Configuration and Defaults: * [`ViewConfig.hpp`](diffhunk://#diff-e403e98244e75d6b7d32864417c37619de6becee73f53415b733073f655f0929R114-R117): Defined new default view configurations for surfaces, portals, volumes, and other elements. (`[[1]](diffhunk://#diff-e403e98244e75d6b7d32864417c37619de6becee73f53415b733073f655f0929R114-R117)`, `[[2]](diffhunk://#diff-e403e98244e75d6b7d32864417c37619de6becee73f53415b733073f655f0929R139-R146)`) ### Codebase Cleanup: * Removed obsolete and redundant code from `ObjVisualization3D.ipp`. (`[Core/include/Acts/Visualization/detail/ObjVisualization3D.ippL1-L179](diffhunk://#diff-29bbb6e8cfcb388c51d1e88237dd0c7d582762d7e05a0ec12278bdc336b0aaa1L1-L179)`) --- .../Acts/Geometry/TrackingGeometry.hpp | 11 + Core/include/Acts/Geometry/TrackingVolume.hpp | 12 + Core/include/Acts/Geometry/Volume.hpp | 2 +- Core/include/Acts/Surfaces/Surface.hpp | 2 +- .../Acts/Visualization/GeometryView3D.hpp | 6 - .../Acts/Visualization/IVisualization3D.hpp | 6 + .../Acts/Visualization/ObjVisualization3D.hpp | 44 ++-- .../Acts/Visualization/PlyVisualization3D.hpp | 4 + .../include/Acts/Visualization/ViewConfig.hpp | 45 +++- .../detail/ObjVisualization3D.ipp | 179 --------------- Core/src/Geometry/TrackingGeometry.cpp | 8 + Core/src/Geometry/TrackingVolume.cpp | 21 ++ Core/src/Visualization/CMakeLists.txt | 5 +- Core/src/Visualization/GeometryView3D.cpp | 10 - Core/src/Visualization/ObjVisualization3D.cpp | 211 ++++++++++++++++++ Examples/Python/python/acts/examples/odd.py | 3 +- Examples/Python/src/Geometry.cpp | 20 +- Examples/Python/src/ModuleEntry.cpp | 2 +- Examples/Python/src/Obj.cpp | 5 + Examples/Python/src/Output.cpp | 8 +- .../Visualization/Visualization3DTests.cpp | 2 + 21 files changed, 366 insertions(+), 240 deletions(-) delete mode 100644 Core/include/Acts/Visualization/detail/ObjVisualization3D.ipp create mode 100644 Core/src/Visualization/ObjVisualization3D.cpp diff --git a/Core/include/Acts/Geometry/TrackingGeometry.hpp b/Core/include/Acts/Geometry/TrackingGeometry.hpp index b0ac658eecd..e8d48c5ae5e 100644 --- a/Core/include/Acts/Geometry/TrackingGeometry.hpp +++ b/Core/include/Acts/Geometry/TrackingGeometry.hpp @@ -145,6 +145,17 @@ class TrackingGeometry { const std::unordered_map& geoIdSurfaceMap() const; + /// Visualize a tracking geometry including substructure + /// @param helper The visualization helper that implement the output + /// @param gctx The geometry context + /// @param viewConfig Global view config + /// @param portalViewConfig View config for portals + /// @param sensitiveViewConfig View configuration for sensitive surfaces + void visualize(IVisualization3D& helper, const GeometryContext& gctx, + const ViewConfig& viewConfig = s_viewVolume, + const ViewConfig& portalViewConfig = s_viewPortal, + const ViewConfig& sensitiveViewConfig = s_viewSensitive) const; + private: // the known world std::shared_ptr m_world; diff --git a/Core/include/Acts/Geometry/TrackingVolume.hpp b/Core/include/Acts/Geometry/TrackingVolume.hpp index 1d1a5b6dc3c..c980314fa6d 100644 --- a/Core/include/Acts/Geometry/TrackingVolume.hpp +++ b/Core/include/Acts/Geometry/TrackingVolume.hpp @@ -26,6 +26,7 @@ #include "Acts/Utilities/BinnedArray.hpp" #include "Acts/Utilities/Logger.hpp" #include "Acts/Utilities/TransformRange.hpp" +#include "Acts/Visualization/ViewConfig.hpp" #include #include @@ -486,6 +487,17 @@ class TrackingVolume : public Volume { /// - positiveFaceXY GlueVolumesDescriptor& glueVolumesDescriptor(); + /// Produces a 3D visualization of this tracking volume + /// @param helper The visualization helper describing the output format + /// @param gctx The geometry context + /// @param viewConfig The view configuration + /// @param portalViewConfig View configuration for portals + /// @param sensitiveViewConfig View configuration for sensitive surfaces + void visualize(IVisualization3D& helper, const GeometryContext& gctx, + const ViewConfig& viewConfig, + const ViewConfig& portalViewConfig, + const ViewConfig& sensitiveViewConfig) const; + private: void connectDenseBoundarySurfaces( MutableTrackingVolumeVector& confinedDenseVolumes); diff --git a/Core/include/Acts/Geometry/Volume.hpp b/Core/include/Acts/Geometry/Volume.hpp index a59451efe09..9217e758444 100644 --- a/Core/include/Acts/Geometry/Volume.hpp +++ b/Core/include/Acts/Geometry/Volume.hpp @@ -122,7 +122,7 @@ class Volume : public GeometryObject { /// @param gctx The geometry context /// @param viewConfig The view configuration void visualize(IVisualization3D& helper, const GeometryContext& gctx, - const ViewConfig& viewConfig = {}) const; + const ViewConfig& viewConfig) const; protected: Transform3 m_transform; diff --git a/Core/include/Acts/Surfaces/Surface.hpp b/Core/include/Acts/Surfaces/Surface.hpp index 3a823671823..79153e8c98e 100644 --- a/Core/include/Acts/Surfaces/Surface.hpp +++ b/Core/include/Acts/Surfaces/Surface.hpp @@ -482,7 +482,7 @@ class Surface : public virtual GeometryObject, const GeometryContext& gctx, const Vector3& position) const = 0; void visualize(IVisualization3D& helper, const GeometryContext& gctx, - const ViewConfig& viewConfig = {}) const; + const ViewConfig& viewConfig = s_viewSurface) const; protected: /// Output Method for std::ostream, to be overloaded by child classes diff --git a/Core/include/Acts/Visualization/GeometryView3D.hpp b/Core/include/Acts/Visualization/GeometryView3D.hpp index 05053de31ef..fed07697a87 100644 --- a/Core/include/Acts/Visualization/GeometryView3D.hpp +++ b/Core/include/Acts/Visualization/GeometryView3D.hpp @@ -31,12 +31,6 @@ class DetectorVolume; class Portal; } // namespace Experimental -static const ViewConfig s_viewSensitive; -static const ViewConfig s_viewPassive; -static const ViewConfig s_viewVolume; -static const ViewConfig s_viewGrid; -static const ViewConfig s_viewLine; - struct GeometryView3D { /// Helper method to draw Polyhedron objects /// diff --git a/Core/include/Acts/Visualization/IVisualization3D.hpp b/Core/include/Acts/Visualization/IVisualization3D.hpp index 734ab3fe129..be8c5b96226 100644 --- a/Core/include/Acts/Visualization/IVisualization3D.hpp +++ b/Core/include/Acts/Visualization/IVisualization3D.hpp @@ -72,6 +72,12 @@ class IVisualization3D { /// Remove all contents of this helper /// virtual void clear() = 0; + + virtual ~IVisualization3D() = default; + + /// Start a new object context + /// @param name The name of the object + virtual void object(const std::string& name) = 0; }; /// Overload of the << operator to facilitate writing to streams. diff --git a/Core/include/Acts/Visualization/ObjVisualization3D.hpp b/Core/include/Acts/Visualization/ObjVisualization3D.hpp index 941e719f870..a9f496ce304 100644 --- a/Core/include/Acts/Visualization/ObjVisualization3D.hpp +++ b/Core/include/Acts/Visualization/ObjVisualization3D.hpp @@ -12,12 +12,8 @@ #include "Acts/Visualization/IVisualization3D.hpp" #include "Acts/Visualization/ViewConfig.hpp" -#include #include -#include -#include #include -#include #include #include @@ -26,14 +22,10 @@ namespace Acts { /// This helper produces output in the OBJ format. Note that colors are not /// supported in this implementation. /// -template class ObjVisualization3D : public IVisualization3D { public: - static_assert(std::is_same_v || std::is_same_v, - "Use either double or float"); - /// Stored value type, should be double or float - using ValueType = T; + using ValueType = double; /// Type of a vertex based on the value type using VertexType = Eigen::Matrix; @@ -77,22 +69,32 @@ class ObjVisualization3D : public IVisualization3D { /// @copydoc Acts::IVisualization3D::clear() void clear() final; + /// Start a new object context with a name + /// @param name The name of the object + void object(const std::string& name) final; + private: + struct Object { + std::string name; + std::vector vertices{}; + std::vector faces{}; + std::vector lines{}; + + /// The object data to be written + /// Map of colors to be written at given index position + std::map lineColors{}; + std::map vertexColors{}; + std::map faceColors{}; + }; + + Object& object(); + const Object& object() const; + /// The output parameters unsigned int m_outputPrecision = 4; double m_outputScalor = 1.; - /// The object data to be written - std::vector m_vertices; - std::vector m_faces; - std::vector m_lines; - /// Map of colors to be written at given index position - std::map m_lineColors; - std::map m_vertexColors; - std::map m_faceColors; -}; -#ifndef DOXYGEN -#include "detail/ObjVisualization3D.ipp" -#endif + std::vector m_objects; +}; } // namespace Acts diff --git a/Core/include/Acts/Visualization/PlyVisualization3D.hpp b/Core/include/Acts/Visualization/PlyVisualization3D.hpp index e2c94481b9c..a4367dc7e02 100644 --- a/Core/include/Acts/Visualization/PlyVisualization3D.hpp +++ b/Core/include/Acts/Visualization/PlyVisualization3D.hpp @@ -60,6 +60,10 @@ class PlyVisualization3D : public IVisualization3D { /// @copydoc Acts::IVisualization3D::clear() void clear() final; + void object(const std::string& /*name*/) final { + // Unimplemented + } + private: std::vector> m_vertices; std::vector m_faces; diff --git a/Core/include/Acts/Visualization/ViewConfig.hpp b/Core/include/Acts/Visualization/ViewConfig.hpp index 0414fa58aba..e2cf7b80618 100644 --- a/Core/include/Acts/Visualization/ViewConfig.hpp +++ b/Core/include/Acts/Visualization/ViewConfig.hpp @@ -10,7 +10,7 @@ #include #include -#include +#include namespace Acts { @@ -49,17 +49,31 @@ struct Color { constexpr Color(double r, double g, double b) : Color{std::array{r, g, b}} {} + private: + constexpr static int hexToInt(std::string_view hex) { + constexpr auto hexCharToInt = [](char c) { + if (c >= '0' && c <= '9') { + return c - '0'; + } else if (c >= 'a' && c <= 'f') { + return c - 'a' + 10; + } else if (c >= 'A' && c <= 'F') { + return c - 'A' + 10; + } else { + throw std::invalid_argument("Invalid hex character"); + } + }; + + int value = 0; + for (char c : hex) { + value = (value << 4) + hexCharToInt(c); + } + return value; + }; + + public: /// Constructor from hex string. The expected format is `#RRGGBB` /// @param hex The hex string constexpr explicit Color(std::string_view hex) { - auto hexToInt = [](std::string_view hexStr) { - int value = 0; - std::stringstream ss; - ss << std::hex << hexStr; - ss >> value; - return value; - }; - if (hex[0] == '#' && hex.size() == 7) { rgb[0] = hexToInt(hex.substr(1, 2)); // Extract R component rgb[1] = hexToInt(hex.substr(3, 2)); // Extract G component @@ -85,7 +99,6 @@ struct Color { /// @param rhs The second color /// @return True if the colors are equal friend bool operator==(const Color& lhs, const Color& rhs) = default; - /// Output stream operator /// @param os The output stream /// @param color The color to be printed @@ -99,6 +112,10 @@ struct Color { std::array rgb{}; }; +constexpr Color s_defaultSurfaceColor{"#0000aa"}; +constexpr Color s_defaultPortalColor{"#308c48"}; +constexpr Color s_defaultVolumColor{"#ffaa00"}; + /// @brief Struct to concentrate all visualization configurations /// in order to harmonize visualization interfaces struct ViewConfig { @@ -120,4 +137,12 @@ struct ViewConfig { std::filesystem::path outputName = std::filesystem::path(""); }; +static const ViewConfig s_viewSurface = {.color = {170, 170, 170}}; +static const ViewConfig s_viewPortal = {.color = Color{"#308c48"}}; +static const ViewConfig s_viewSensitive = {.color = {0, 180, 240}}; +static const ViewConfig s_viewPassive = {.color = {240, 280, 0}}; +static const ViewConfig s_viewVolume = {.color = {220, 220, 0}}; +static const ViewConfig s_viewGrid = {.color = {220, 0, 0}}; +static const ViewConfig s_viewLine = {.color = {0, 0, 220}}; + } // namespace Acts diff --git a/Core/include/Acts/Visualization/detail/ObjVisualization3D.ipp b/Core/include/Acts/Visualization/detail/ObjVisualization3D.ipp deleted file mode 100644 index 354ef518bef..00000000000 --- a/Core/include/Acts/Visualization/detail/ObjVisualization3D.ipp +++ /dev/null @@ -1,179 +0,0 @@ -// 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/. - -template -void ObjVisualization3D::vertex(const Vector3& vtx, Color color) { - m_vertexColors[m_vertices.size()] = color; - m_vertices.push_back(vtx.template cast()); -} - -template -void ObjVisualization3D::line(const Vector3& a, const Vector3& b, - Color color) { - if (color != Color{0, 0, 0}) { - m_lineColors[m_lines.size()] = color; - } - // not implemented - vertex(a, color); - vertex(b, color); - m_lines.push_back({m_vertices.size() - 2, m_vertices.size() - 1}); -} - -template -void ObjVisualization3D::face(const std::vector& vtxs, - Color color) { - if (color != Color{0, 0, 0}) { - m_faceColors[m_faces.size()] = color; - } - FaceType idxs; - idxs.reserve(vtxs.size()); - for (const auto& vtx : vtxs) { - vertex(vtx, color); - idxs.push_back(m_vertices.size() - 1); - } - m_faces.push_back(std::move(idxs)); -} - -template -void ObjVisualization3D::faces(const std::vector& vtxs, - const std::vector& faces, - Color color) { - // No faces given - call the face() method - if (faces.empty()) { - face(vtxs, color); - } else { - if (color != Color{0, 0, 0}) { - m_faceColors[m_faces.size()] = color; - } - auto vtxoffs = m_vertices.size(); - if (color != Color{0, 0, 0}) { - m_vertexColors[m_vertices.size()] = color; - } - m_vertices.insert(m_vertices.end(), vtxs.begin(), vtxs.end()); - for (const auto& face : faces) { - if (face.size() == 2) { - m_lines.push_back({face[0] + vtxoffs, face[2] + vtxoffs}); - } else { - FaceType rawFace = face; - std::transform(rawFace.begin(), rawFace.end(), rawFace.begin(), - [&](std::size_t& iv) { return (iv + vtxoffs); }); - m_faces.push_back(rawFace); - } - } - } -} - -template -void ObjVisualization3D::write(const std::filesystem::path& path) const { - std::ofstream os; - std::filesystem::path objectpath = path; - if (!objectpath.has_extension()) { - objectpath.replace_extension(std::filesystem::path("obj")); - } - os.open(std::filesystem::absolute(objectpath).string()); - std::filesystem::path mtlpath = objectpath; - mtlpath.replace_extension(std::filesystem::path("mtl")); - - const std::string mtlpathString = std::filesystem::absolute(mtlpath).string(); - os << "mtllib " << mtlpathString << "\n"; - std::ofstream mtlos; - mtlos.open(mtlpathString); - - write(os, mtlos); - os.close(); - mtlos.close(); -} - -template -void ObjVisualization3D::write(std::ostream& os) const { - std::stringstream sterile; - write(os, sterile); -} - -template -void ObjVisualization3D::write(std::ostream& os, std::ostream& mos) const { - std::map materials; - - auto mixColor = [&](const Color& color) -> std::string { - std::string materialName; - materialName = "material_"; - materialName += std::to_string(color[0]) + std::string("_"); - materialName += std::to_string(color[1]) + std::string("_"); - materialName += std::to_string(color[2]); - - if (!materials.contains(materialName)) { - mos << "newmtl " << materialName << "\n"; - std::vector shadings = {"Ka", "Kd", "Ks"}; - for (const auto& shd : shadings) { - mos << shd << " " << std::to_string(color[0] / 256.) << " "; - mos << std::to_string(color[1] / 256.) << " "; - mos << std::to_string(color[2] / 256.) << " " - << "\n"; - } - mos << "\n"; - } - return std::string("usemtl ") + materialName; - }; - - std::size_t iv = 0; - Color lastVertexColor = {0, 0, 0}; - for (const VertexType& vtx : m_vertices) { - if (m_vertexColors.contains(iv)) { - auto color = m_vertexColors.find(iv)->second; - if (color != lastVertexColor) { - os << mixColor(color) << "\n"; - lastVertexColor = color; - } - } - - os << "v " << std::setprecision(m_outputPrecision) - << m_outputScalor * vtx.x() << " " << m_outputScalor * vtx.y() << " " - << m_outputScalor * vtx.z() << "\n"; - ++iv; - } - std::size_t il = 0; - Color lastLineColor = {0, 0, 0}; - for (const LineType& ln : m_lines) { - if (m_lineColors.contains(il)) { - auto color = m_lineColors.find(il)->second; - if (color != lastLineColor) { - os << mixColor(color) << "\n"; - lastLineColor = color; - } - } - os << "l " << ln.first + 1 << " " << ln.second + 1 << "\n"; - ++il; - } - std::size_t is = 0; - Color lastFaceColor = {0, 0, 0}; - for (const FaceType& fc : m_faces) { - if (m_faceColors.contains(is)) { - auto color = m_faceColors.find(is)->second; - if (color != lastFaceColor) { - os << mixColor(color) << "\n"; - lastFaceColor = color; - } - } - os << "f"; - for (auto fi : fc) { - os << " " << fi + 1; - } - os << "\n"; - ++is; - } -} - -template -void ObjVisualization3D::clear() { - m_vertices.clear(); - m_faces.clear(); - m_lines.clear(); - m_lineColors.clear(); - m_vertexColors.clear(); - m_faceColors.clear(); -} diff --git a/Core/src/Geometry/TrackingGeometry.cpp b/Core/src/Geometry/TrackingGeometry.cpp index b68d788d08b..f9207afbd18 100644 --- a/Core/src/Geometry/TrackingGeometry.cpp +++ b/Core/src/Geometry/TrackingGeometry.cpp @@ -85,3 +85,11 @@ const std::unordered_map& Acts::TrackingGeometry::geoIdSurfaceMap() const { return m_surfacesById; } + +void Acts::TrackingGeometry::visualize( + IVisualization3D& helper, const GeometryContext& gctx, + const ViewConfig& viewConfig, const ViewConfig& portalViewConfig, + const ViewConfig& sensitiveViewConfig) const { + highestTrackingVolume()->visualize(helper, gctx, viewConfig, portalViewConfig, + sensitiveViewConfig); +} diff --git a/Core/src/Geometry/TrackingVolume.cpp b/Core/src/Geometry/TrackingVolume.cpp index 9bcf9cc969f..ad2c939fcbd 100644 --- a/Core/src/Geometry/TrackingVolume.cpp +++ b/Core/src/Geometry/TrackingVolume.cpp @@ -723,4 +723,25 @@ void TrackingVolume::addSurface(std::shared_ptr surface) { m_surfaces.push_back(std::move(surface)); } +void TrackingVolume::visualize(IVisualization3D& helper, + const GeometryContext& gctx, + const ViewConfig& viewConfig, + const ViewConfig& portalViewConfig, + const ViewConfig& sensitiveViewConfig) const { + helper.object(volumeName()); + Volume::visualize(helper, gctx, viewConfig); + + if (!surfaces().empty()) { + helper.object(volumeName() + "_sensitives"); + } + for (const auto& surface : surfaces()) { + surface.visualize(helper, gctx, sensitiveViewConfig); + } + + for (const auto& child : volumes()) { + child.visualize(helper, gctx, viewConfig, portalViewConfig, + sensitiveViewConfig); + } +} + } // namespace Acts diff --git a/Core/src/Visualization/CMakeLists.txt b/Core/src/Visualization/CMakeLists.txt index 5d4f916af1c..567b1e1e236 100644 --- a/Core/src/Visualization/CMakeLists.txt +++ b/Core/src/Visualization/CMakeLists.txt @@ -1 +1,4 @@ -target_sources(ActsCore PRIVATE GeometryView3D.cpp EventDataView3D.cpp) +target_sources( + ActsCore + PRIVATE GeometryView3D.cpp EventDataView3D.cpp ObjVisualization3D.cpp +) diff --git a/Core/src/Visualization/GeometryView3D.cpp b/Core/src/Visualization/GeometryView3D.cpp index 17b7b5c7df9..f16d9e809cc 100644 --- a/Core/src/Visualization/GeometryView3D.cpp +++ b/Core/src/Visualization/GeometryView3D.cpp @@ -26,7 +26,6 @@ #include "Acts/Surfaces/RadialBounds.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Surfaces/SurfaceArray.hpp" -#include "Acts/Utilities/BinnedArray.hpp" #include "Acts/Utilities/BinningType.hpp" #include "Acts/Utilities/IAxis.hpp" #include "Acts/Utilities/UnitVectors.hpp" @@ -36,18 +35,9 @@ #include #include #include -#include #include #include -namespace Acts::Experimental { -ViewConfig s_viewSensitive = {.color = {0, 180, 240}}; -ViewConfig s_viewPassive = {.color = {240, 280, 0}}; -ViewConfig s_viewVolume = {.color = {220, 220, 0}}; -ViewConfig s_viewGrid = {.color = {220, 0, 0}}; -ViewConfig s_viewLine = {.color = {0, 0, 220}}; -} // namespace Acts::Experimental - void Acts::GeometryView3D::drawPolyhedron(IVisualization3D& helper, const Polyhedron& polyhedron, const ViewConfig& viewConfig) { diff --git a/Core/src/Visualization/ObjVisualization3D.cpp b/Core/src/Visualization/ObjVisualization3D.cpp new file mode 100644 index 00000000000..777e883b3d3 --- /dev/null +++ b/Core/src/Visualization/ObjVisualization3D.cpp @@ -0,0 +1,211 @@ +// 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/Visualization/ObjVisualization3D.hpp" + +#include +#include +#include + +namespace Acts { + +void ObjVisualization3D::vertex(const Vector3& vtx, Color color) { + auto& o = object(); + o.vertexColors[o.vertices.size()] = color; + o.vertices.push_back(vtx.template cast()); +} + +void ObjVisualization3D::line(const Vector3& a, const Vector3& b, Color color) { + auto& o = object(); + if (color != Color{0, 0, 0}) { + o.lineColors[o.lines.size()] = color; + } + // not implemented + vertex(a, color); + vertex(b, color); + o.lines.push_back({o.vertices.size() - 2, o.vertices.size() - 1}); +} + +void ObjVisualization3D::face(const std::vector& vtxs, Color color) { + auto& o = object(); + if (color != Color{0, 0, 0}) { + o.faceColors[o.faces.size()] = color; + } + FaceType idxs; + idxs.reserve(vtxs.size()); + for (const auto& vtx : vtxs) { + vertex(vtx, color); + idxs.push_back(o.vertices.size() - 1); + } + o.faces.push_back(std::move(idxs)); +} + +void ObjVisualization3D::faces(const std::vector& vtxs, + const std::vector& faces, + Color color) { + auto& o = object(); + // No faces given - call the face() method + if (faces.empty()) { + face(vtxs, color); + } else { + if (color != Color{0, 0, 0}) { + o.faceColors[o.faces.size()] = color; + } + auto vtxoffs = o.vertices.size(); + if (color != Color{0, 0, 0}) { + o.vertexColors[o.vertices.size()] = color; + } + o.vertices.insert(o.vertices.end(), vtxs.begin(), vtxs.end()); + for (const auto& face : faces) { + if (face.size() == 2) { + o.lines.push_back({face[0] + vtxoffs, face[2] + vtxoffs}); + } else { + FaceType rawFace; + std::ranges::transform( + face, std::back_inserter(rawFace), + [&](unsigned long iv) { return (iv + vtxoffs); }); + o.faces.push_back(rawFace); + } + } + } +} + +void ObjVisualization3D::write(const std::filesystem::path& path) const { + std::ofstream os; + std::filesystem::path objectpath = path; + if (!objectpath.has_extension()) { + objectpath.replace_extension(std::filesystem::path("obj")); + } + os.open(std::filesystem::absolute(objectpath).string()); + std::filesystem::path mtlpath = objectpath; + mtlpath.replace_extension(std::filesystem::path("mtl")); + + const std::string mtlpathString = std::filesystem::absolute(mtlpath).string(); + os << "mtllib " << mtlpathString << "\n"; + std::ofstream mtlos; + mtlos.open(mtlpathString); + + write(os, mtlos); + os.close(); + mtlos.close(); +} + +void ObjVisualization3D::write(std::ostream& os) const { + std::stringstream sterile; + write(os, sterile); +} + +void ObjVisualization3D::write(std::ostream& os, std::ostream& mos) const { + std::map> materials; + + auto mixColor = [&](const Color& color) { + std::string materialName; + materialName = "material_"; + materialName += std::to_string(color[0]) + std::string("_"); + materialName += std::to_string(color[1]) + std::string("_"); + materialName += std::to_string(color[2]); + + if (!materials.contains(materialName)) { + mos << "newmtl " << materialName << "\n"; + std::vector shadings = {"Ka", "Kd", "Ks"}; + for (const auto& shd : shadings) { + mos << shd << " " << std::to_string(color[0] / 256.) << " "; + mos << std::to_string(color[1] / 256.) << " "; + mos << std::to_string(color[2] / 256.) << " " << "\n"; + } + mos << "\n"; + } + return std::string("usemtl ") + materialName; + }; + + std::size_t vertexOffset = 0; + for (const auto& o : m_objects) { + if (!o.name.empty()) { + os << "o " << o.name << "\n"; + } + + std::size_t iv = 0; + Color lastVertexColor = {0, 0, 0}; + for (const VertexType& vtx : o.vertices) { + if (o.vertexColors.contains(iv)) { + auto color = o.vertexColors.find(iv)->second; + if (color != lastVertexColor) { + os << mixColor(color) << "\n"; + lastVertexColor = color; + } + } + + os << "v " << std::setprecision(m_outputPrecision) + << m_outputScalor * vtx.x() << " " << m_outputScalor * vtx.y() << " " + << m_outputScalor * vtx.z() << "\n"; + ++iv; + } + std::size_t il = 0; + Color lastLineColor = {0, 0, 0}; + for (const auto& [start, end] : o.lines) { + if (o.lineColors.contains(il)) { + auto color = o.lineColors.find(il)->second; + if (color != lastLineColor) { + os << mixColor(color) << "\n"; + lastLineColor = color; + } + } + os << "l " << vertexOffset + start + 1 << " " << vertexOffset + end + 1 + << "\n"; + ++il; + } + std::size_t is = 0; + Color lastFaceColor = {0, 0, 0}; + for (const FaceType& fc : o.faces) { + if (o.faceColors.contains(is)) { + auto color = o.faceColors.find(is)->second; + if (color != lastFaceColor) { + os << mixColor(color) << "\n"; + lastFaceColor = color; + } + } + os << "f"; + for (std::size_t fi : fc) { + os << " " << vertexOffset + fi + 1; + } + os << "\n"; + ++is; + } + + vertexOffset += iv; + } +} + +void ObjVisualization3D::clear() { + m_objects.clear(); +} + +void ObjVisualization3D::object(const std::string& name) { + if (name.empty()) { + throw std::invalid_argument{"Object name can not be empty"}; + } + m_objects.push_back(Object{.name = name}); +} + +ObjVisualization3D::Object& ObjVisualization3D::object() { + if (m_objects.empty()) { + m_objects.push_back(Object{.name = ""}); + } + + return m_objects.back(); +} + +const ObjVisualization3D::Object& ObjVisualization3D::object() const { + if (m_objects.empty()) { + throw std::runtime_error{"No objects present"}; + } + + return m_objects.back(); +} + +} // namespace Acts diff --git a/Examples/Python/python/acts/examples/odd.py b/Examples/Python/python/acts/examples/odd.py index 4e1efb1c783..a28058af78b 100644 --- a/Examples/Python/python/acts/examples/odd.py +++ b/Examples/Python/python/acts/examples/odd.py @@ -76,8 +76,9 @@ def getOpenDataDetector( } def geoid_hook(geoid, surface): + gctx = acts.GeometryContext() if geoid.volume() in volumeRadiusCutsMap: - r = math.sqrt(surface.center()[0] ** 2 + surface.center()[1] ** 2) + r = math.sqrt(surface.center(gctx)[0] ** 2 + surface.center(gctx)[1] ** 2) geoid.setExtra(1) for cut in volumeRadiusCutsMap[geoid.volume()]: diff --git a/Examples/Python/src/Geometry.cpp b/Examples/Python/src/Geometry.cpp index 51188ee93e4..05484845465 100644 --- a/Examples/Python/src/Geometry.cpp +++ b/Examples/Python/src/Geometry.cpp @@ -37,6 +37,7 @@ #include "Acts/Surfaces/SurfaceArray.hpp" #include "Acts/Utilities/Helpers.hpp" #include "Acts/Utilities/RangeXD.hpp" +#include "Acts/Visualization/ViewConfig.hpp" #include "ActsExamples/Geometry/VolumeAssociationTest.hpp" #include @@ -109,13 +110,12 @@ void addGeometry(Context& ctx) { { py::class_>(m, "Surface") + // Can't bind directly because GeometryObject is virtual base of Surface .def("geometryId", - [](Acts::Surface& self) { return self.geometryId(); }) - .def("center", - [](Acts::Surface& self) { - return self.center(Acts::GeometryContext{}); - }) - .def("type", [](Acts::Surface& self) { return self.type(); }); + [](const Surface& self) { return self.geometryId(); }) + .def("center", &Surface::center) + .def("type", &Surface::type) + .def("visualize", &Surface::visualize); } { @@ -167,7 +167,11 @@ void addGeometry(Context& ctx) { }) .def_property_readonly( "highestTrackingVolume", - &Acts::TrackingGeometry::highestTrackingVolumePtr); + &Acts::TrackingGeometry::highestTrackingVolumePtr) + .def("visualize", &Acts::TrackingGeometry::visualize, py::arg("helper"), + py::arg("gctx"), py::arg("viewConfig") = s_viewVolume, + py::arg("portalViewConfig") = s_viewPortal, + py::arg("sensitiveViewConfig") = s_viewSensitive); } { @@ -287,7 +291,7 @@ void addExperimentalGeometry(Context& ctx) { for (const auto& surface : smap) { auto gid = surface->geometryId(); // Exclusion criteria - if (sensitiveOnly and gid.sensitive() == 0) { + if (sensitiveOnly && gid.sensitive() == 0) { continue; }; surfaceVolumeLayerMap[gid.volume()][gid.layer()].push_back(surface); diff --git a/Examples/Python/src/ModuleEntry.cpp b/Examples/Python/src/ModuleEntry.cpp index 77348edaf1c..1a0e27db907 100644 --- a/Examples/Python/src/ModuleEntry.cpp +++ b/Examples/Python/src/ModuleEntry.cpp @@ -120,6 +120,7 @@ PYBIND11_MODULE(ActsPythonBindings, m) { addAlgebra(ctx); addBinning(ctx); addEventData(ctx); + addOutput(ctx); addPropagation(ctx); addGeometryBuildingGen1(ctx); @@ -128,7 +129,6 @@ PYBIND11_MODULE(ActsPythonBindings, m) { addMagneticField(ctx); addMaterial(ctx); - addOutput(ctx); addDetector(ctx); addExampleAlgorithms(ctx); addInput(ctx); diff --git a/Examples/Python/src/Obj.cpp b/Examples/Python/src/Obj.cpp index 5ca2df40033..09acb0c53b6 100644 --- a/Examples/Python/src/Obj.cpp +++ b/Examples/Python/src/Obj.cpp @@ -6,6 +6,7 @@ // 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/Visualization/IVisualization3D.hpp" #include #include #include @@ -19,6 +20,7 @@ #include #include +#include namespace py = pybind11; using namespace pybind11::literals; @@ -87,5 +89,8 @@ void addObj(Context& ctx) { obj.write(fileName); }); } + + py::class_(m, "ObjVisualization3D") + .def(py::init<>()); } } // namespace Acts::Python diff --git a/Examples/Python/src/Output.cpp b/Examples/Python/src/Output.cpp index b53ba905700..11472d8e203 100644 --- a/Examples/Python/src/Output.cpp +++ b/Examples/Python/src/Output.cpp @@ -10,6 +10,7 @@ #include "Acts/Geometry/GeometryHierarchyMap.hpp" #include "Acts/Plugins/Python/Utilities.hpp" #include "Acts/Utilities/Logger.hpp" +#include "Acts/Visualization/IVisualization3D.hpp" #include "Acts/Visualization/ViewConfig.hpp" #include "ActsExamples/Digitization/DigitizationConfig.hpp" #include "ActsExamples/Framework/ProcessCode.hpp" @@ -57,6 +58,7 @@ #include #include +#include namespace Acts { class TrackingGeometry; @@ -130,10 +132,14 @@ void addOutput(Context& ctx) { .def(py::init<>()) .def(py::init()) .def(py::init()) - .def(py::init()) + .def(py::init()) .def_readonly("rgb", &Color::rgb); } + py::class_(m, "IVisualization3D") + .def("write", py::overload_cast( + &IVisualization3D::write, py::const_)); + { using Writer = ActsExamples::ObjTrackingGeometryWriter; auto w = py::class_>( diff --git a/Tests/UnitTests/Core/Visualization/Visualization3DTests.cpp b/Tests/UnitTests/Core/Visualization/Visualization3DTests.cpp index 58958bbbe6c..ea3e810d119 100644 --- a/Tests/UnitTests/Core/Visualization/Visualization3DTests.cpp +++ b/Tests/UnitTests/Core/Visualization/Visualization3DTests.cpp @@ -335,6 +335,8 @@ BOOST_AUTO_TEST_CASE(ColorTests) { BOOST_CHECK_EQUAL(grey, Color(std::array{128 / 255.0, 128 / 255.0, 128 / 255.0})); BOOST_CHECK_EQUAL(grey, Color(128 / 255.0, 128 / 255.0, 128 / 255.0)); + + static_assert(Color{"#0000ff"} == Color(0, 0, 255)); } BOOST_AUTO_TEST_SUITE_END() From 39c6acac6dee2504fa8213c2d37a3ad42aecb3d4 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Mon, 7 Oct 2024 20:21:21 +0200 Subject: [PATCH 07/18] refactor(geo): Teach ProtoLayer to respect local coordinate system (#3697) Needed for https://github.com/acts-project/acts/issues/3502 --- This pull request introduces several enhancements and new features to the `ProtoLayer` struct and its associated tests in the `Core` and `Tests` directories. The key changes include the addition of a local transform, multiple constructors, and new test cases to validate the functionality. ### Enhancements to `ProtoLayer` struct: * Added a new member `transform` to the `ProtoLayer` struct and updated its constructors to accept a `Transform3` parameter with a default value of `Transform3::Identity()`. (`Core/include/Acts/Geometry/ProtoLayer.hpp`, `Core/src/Geometry/ProtoLayer.cpp`) [[1]](diffhunk://#diff-1451530599ae50cdf187f035045a620fe857ff4abcb25c7a0eefc33189ef3260R36-R38) [[2]](diffhunk://#diff-1451530599ae50cdf187f035045a620fe857ff4abcb25c7a0eefc33189ef3260R47-R50) [[3]](diffhunk://#diff-1451530599ae50cdf187f035045a620fe857ff4abcb25c7a0eefc33189ef3260R60-R76) [[4]](diffhunk://#diff-4d780d37fe19501d1f964a2d67f4a654c4365f31f736d0c9653a169f1637f29eL16-R43) [[5]](diffhunk://#diff-4d780d37fe19501d1f964a2d67f4a654c4365f31f736d0c9653a169f1637f29eL81-R95) * Implemented a friend function for the output stream operator to facilitate easy printing of `ProtoLayer` objects. (`Core/include/Acts/Geometry/ProtoLayer.hpp`) ### Codebase simplification: * Removed unused `#include` directives from `ProtoLayer.cpp` to clean up the code. (`Core/src/Geometry/ProtoLayer.cpp`) ### Enhancements to unit tests: * Added new test cases in `ProtoLayerTests.cpp` to validate the behavior of `ProtoLayer` with different transformations. These changes enhance the flexibility and functionality of the `ProtoLayer` struct and ensure that the new features are thoroughly tested. --- Core/include/Acts/Geometry/ProtoLayer.hpp | 32 +++- Core/src/Geometry/ProtoLayer.cpp | 34 ++-- .../Core/Geometry/ProtoLayerTests.cpp | 161 +++++++++++++++--- 3 files changed, 184 insertions(+), 43 deletions(-) diff --git a/Core/include/Acts/Geometry/ProtoLayer.hpp b/Core/include/Acts/Geometry/ProtoLayer.hpp index d12e674918d..1e5d95dc025 100644 --- a/Core/include/Acts/Geometry/ProtoLayer.hpp +++ b/Core/include/Acts/Geometry/ProtoLayer.hpp @@ -33,6 +33,9 @@ struct ProtoLayer { /// The envelope parameters ExtentEnvelope envelope = ExtentEnvelope::Zero(); + /// The local transform + Transform3 transform = Transform3::Identity(); + /// Constructor /// /// Loops over a provided vector of surface and calculates the various @@ -41,8 +44,10 @@ struct ProtoLayer { /// /// @param gctx The current geometry context object, e.g. alignment /// @param surfaces The vector of surfaces to consider + /// @param transformIn The local transform to evaluate the sizing in ProtoLayer(const GeometryContext& gctx, - const std::vector& surfaces); + const std::vector& surfaces, + const Transform3& transformIn = Transform3::Identity()); /// Constructor /// @@ -52,8 +57,23 @@ struct ProtoLayer { /// /// @param gctx The current geometry context object, e.g. alignment /// @param surfaces The vector of surfaces to consider + /// @param transformIn The local transform to evaluate the sizing in ProtoLayer(const GeometryContext& gctx, - const std::vector>& surfaces); + const std::vector>& surfaces, + const Transform3& transformIn = Transform3::Identity()); + + /// Constructor + /// + /// Loops over a provided vector of surface and calculates the various + /// min/max values in one go. Also takes into account the thickness + /// of an associated DetectorElement, if it exists. + /// + /// @param gctx The current geometry context object, e.g. alignment + /// @param surfaces The vector of surfaces to consider + /// @param transformIn The local transform to evaluate the sizing in + ProtoLayer(const GeometryContext& gctx, + const std::vector>& surfaces, + const Transform3& transformIn = Transform3::Identity()); ProtoLayer() = default; @@ -81,6 +101,14 @@ struct ProtoLayer { /// @param sl the input ostream std::ostream& toStream(std::ostream& sl) const; + /// Output stream operator + /// @param sl the input ostream + /// @param pl the ProtoLayer to be printed + /// @return the output ostream + friend std::ostream& operator<<(std::ostream& sl, const ProtoLayer& pl) { + return pl.toStream(sl); + } + /// Give access to the surfaces used/assigned to the ProtoLayer const std::vector& surfaces() const; diff --git a/Core/src/Geometry/ProtoLayer.cpp b/Core/src/Geometry/ProtoLayer.cpp index 659b34b1168..c8f190d4293 100644 --- a/Core/src/Geometry/ProtoLayer.cpp +++ b/Core/src/Geometry/ProtoLayer.cpp @@ -13,26 +13,34 @@ #include "Acts/Surfaces/RegularSurface.hpp" #include "Acts/Utilities/Helpers.hpp" -#include -#include -#include -#include - using Acts::VectorHelpers::perp; using Acts::VectorHelpers::phi; namespace Acts { ProtoLayer::ProtoLayer(const GeometryContext& gctx, - const std::vector& surfaces) - : m_surfaces(surfaces) { + const std::vector& surfaces, + const Transform3& transformIn) + : transform(transformIn), m_surfaces(surfaces) { measure(gctx, surfaces); } ProtoLayer::ProtoLayer( const GeometryContext& gctx, - const std::vector>& surfaces) - : m_surfaces(unpack_shared_vector(surfaces)) { + const std::vector>& surfaces, + const Transform3& transformIn) + : transform(transformIn), m_surfaces(unpack_shared_vector(surfaces)) { + measure(gctx, m_surfaces); +} + +ProtoLayer::ProtoLayer(const GeometryContext& gctx, + const std::vector>& surfaces, + const Transform3& transformIn) + : transform(transformIn) { + m_surfaces.reserve(surfaces.size()); + for (const auto& sf : surfaces) { + m_surfaces.push_back(sf.get()); + } measure(gctx, m_surfaces); } @@ -78,15 +86,13 @@ void ProtoLayer::measure(const GeometryContext& gctx, double thickness = element->thickness(); // We need a translation along and opposite half thickness Vector3 sfNormal = regSurface->normal(gctx, sf->center(gctx)); - std::vector deltaT = {-0.5 * thickness, 0.5 * thickness}; - for (const auto& dT : deltaT) { - Transform3 dtransform = Transform3::Identity(); - dtransform.pretranslate(dT * sfNormal); + for (const auto& dT : {-0.5 * thickness, 0.5 * thickness}) { + Transform3 dtransform = transform * Translation3{dT * sfNormal}; extent.extend(sfPolyhedron.extent(dtransform)); } continue; } - extent.extend(sfPolyhedron.extent()); + extent.extend(sfPolyhedron.extent(transform)); } } diff --git a/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp b/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp index cbace6a6559..efde97d9b41 100644 --- a/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp +++ b/Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp @@ -9,30 +9,33 @@ #include #include "Acts/Definitions/Algebra.hpp" +#include "Acts/Definitions/Units.hpp" +#include "Acts/Geometry/DetectorElementBase.hpp" #include "Acts/Geometry/Extent.hpp" #include "Acts/Geometry/GeometryContext.hpp" #include "Acts/Geometry/ProtoLayer.hpp" #include "Acts/Surfaces/PlaneSurface.hpp" #include "Acts/Surfaces/RectangleBounds.hpp" #include "Acts/Surfaces/Surface.hpp" +#include "Acts/Tests/CommonHelpers/DetectorElementStub.hpp" #include "Acts/Tests/CommonHelpers/FloatComparisons.hpp" #include "Acts/Utilities/BinningType.hpp" #include "Acts/Utilities/RangeXD.hpp" -#include #include #include #include #include -#include #include namespace Acts::Test::Layers { +GeometryContext tgContext = GeometryContext(); + BOOST_AUTO_TEST_SUITE(Geometry) BOOST_AUTO_TEST_CASE(ProtoLayerTests) { - GeometryContext tgContext = GeometryContext(); + using enum BinningValue; // Create a proto layer with 4 surfaces on the x/y grid auto recBounds = std::make_shared(3., 6.); @@ -105,20 +108,20 @@ BOOST_AUTO_TEST_CASE(ProtoLayerTests) { // Test 1 - identity transform auto protoLayer = createProtoLayer(Transform3::Identity()); - CHECK_CLOSE_ABS(protoLayer.range(BinningValue::binX), 12., 1e-8); - CHECK_CLOSE_ABS(protoLayer.medium(BinningValue::binX), 0., 1e-8); - CHECK_CLOSE_ABS(protoLayer.min(BinningValue::binX), -6., 1e-8); - CHECK_CLOSE_ABS(protoLayer.max(BinningValue::binX), 6., 1e-8); - CHECK_CLOSE_ABS(protoLayer.range(BinningValue::binY), 6., 1e-8); - CHECK_CLOSE_ABS(protoLayer.medium(BinningValue::binY), 0., 1e-8); - CHECK_CLOSE_ABS(protoLayer.min(BinningValue::binY), -3., 1e-8); - CHECK_CLOSE_ABS(protoLayer.max(BinningValue::binY), 3., 1e-8); - CHECK_CLOSE_ABS(protoLayer.range(BinningValue::binZ), 12., 1e-8); - CHECK_CLOSE_ABS(protoLayer.medium(BinningValue::binZ), 0., 1e-8); - CHECK_CLOSE_ABS(protoLayer.min(BinningValue::binZ), -6., 1e-8); - CHECK_CLOSE_ABS(protoLayer.max(BinningValue::binZ), 6., 1e-8); - CHECK_CLOSE_ABS(protoLayer.max(BinningValue::binR), std::hypot(3, 6), 1e-8); - CHECK_CLOSE_ABS(protoLayer.min(BinningValue::binR), 3., 1e-8); + CHECK_CLOSE_ABS(protoLayer.range(binX), 12., 1e-8); + CHECK_CLOSE_ABS(protoLayer.medium(binX), 0., 1e-8); + CHECK_CLOSE_ABS(protoLayer.min(binX), -6., 1e-8); + CHECK_CLOSE_ABS(protoLayer.max(binX), 6., 1e-8); + CHECK_CLOSE_ABS(protoLayer.range(binY), 6., 1e-8); + CHECK_CLOSE_ABS(protoLayer.medium(binY), 0., 1e-8); + CHECK_CLOSE_ABS(protoLayer.min(binY), -3., 1e-8); + CHECK_CLOSE_ABS(protoLayer.max(binY), 3., 1e-8); + CHECK_CLOSE_ABS(protoLayer.range(binZ), 12., 1e-8); + CHECK_CLOSE_ABS(protoLayer.medium(binZ), 0., 1e-8); + CHECK_CLOSE_ABS(protoLayer.min(binZ), -6., 1e-8); + CHECK_CLOSE_ABS(protoLayer.max(binZ), 6., 1e-8); + CHECK_CLOSE_ABS(protoLayer.max(binR), std::hypot(3, 6), 1e-8); + CHECK_CLOSE_ABS(protoLayer.min(binR), 3., 1e-8); // Test 1a @@ -127,16 +130,15 @@ BOOST_AUTO_TEST_CASE(ProtoLayerTests) { auto protoLayerRot = createProtoLayer(AngleAxis3(-0.345, Vector3::UnitZ()) * Transform3::Identity()); - BOOST_CHECK_NE(protoLayer.min(BinningValue::binX), -6.); - CHECK_CLOSE_ABS(protoLayerRot.medium(BinningValue::binX), 0., 1e-8); - CHECK_CLOSE_ABS(protoLayerRot.medium(BinningValue::binY), 0., 1e-8); - CHECK_CLOSE_ABS(protoLayerRot.range(BinningValue::binZ), 12., 1e-8); - CHECK_CLOSE_ABS(protoLayerRot.medium(BinningValue::binZ), 0., 1e-8); - CHECK_CLOSE_ABS(protoLayerRot.min(BinningValue::binZ), -6., 1e-8); - CHECK_CLOSE_ABS(protoLayerRot.max(BinningValue::binZ), 6., 1e-8); - CHECK_CLOSE_ABS(protoLayerRot.min(BinningValue::binR), 3., 1e-8); - CHECK_CLOSE_ABS(protoLayerRot.max(BinningValue::binR), std::hypot(3, 6), - 1e-8); + BOOST_CHECK_NE(protoLayer.min(binX), -6.); + CHECK_CLOSE_ABS(protoLayerRot.medium(binX), 0., 1e-8); + CHECK_CLOSE_ABS(protoLayerRot.medium(binY), 0., 1e-8); + CHECK_CLOSE_ABS(protoLayerRot.range(binZ), 12., 1e-8); + CHECK_CLOSE_ABS(protoLayerRot.medium(binZ), 0., 1e-8); + CHECK_CLOSE_ABS(protoLayerRot.min(binZ), -6., 1e-8); + CHECK_CLOSE_ABS(protoLayerRot.max(binZ), 6., 1e-8); + CHECK_CLOSE_ABS(protoLayerRot.min(binR), 3., 1e-8); + CHECK_CLOSE_ABS(protoLayerRot.max(binR), std::hypot(3, 6), 1e-8); std::stringstream sstream; protoLayerRot.toStream(sstream); @@ -155,6 +157,111 @@ Extent in space : BOOST_CHECK_EQUAL(sstream.str(), oString); } +BOOST_AUTO_TEST_CASE(OrientedLayer) { + using enum BinningValue; + using namespace Acts::UnitLiterals; + + Transform3 base = Transform3::Identity(); + + auto recBounds = std::make_shared(3_mm, 6_mm); + + std::vector> detectorElements; + + auto makeFan = [&](double yrot, double thickness = 0) { + detectorElements.clear(); + + std::size_t nSensors = 8; + double deltaPhi = 2 * M_PI / nSensors; + double r = 20_mm; + std::vector> surfaces; + for (std::size_t i = 0; i < nSensors; i++) { + // Create a fan of sensors + + Transform3 trf = base * AngleAxis3{yrot, Vector3::UnitY()} * + AngleAxis3{deltaPhi * i, Vector3::UnitZ()} * + Translation3(Vector3::UnitX() * r); + + auto& element = detectorElements.emplace_back( + std::make_unique(trf, recBounds, thickness)); + + surfaces.push_back(element->surface().getSharedPtr()); + } + return surfaces; + }; + + std::vector> surfaces = makeFan(0_degree); + + ProtoLayer protoLayer(tgContext, surfaces); + + BOOST_CHECK_EQUAL(protoLayer.surfaces().size(), 8); + BOOST_CHECK_CLOSE(protoLayer.min(binX), -23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binX), 23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binY), -23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binY), 23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binZ), 0_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binZ), 0_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binR), 17_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binR), 23.769728648_mm, 1e-8); + + surfaces = makeFan(45_degree); + + // Do NOT provide rotation matrix: sizing will be affected + protoLayer = {tgContext, surfaces}; + + BOOST_CHECK_EQUAL(protoLayer.surfaces().size(), 8); + BOOST_CHECK_CLOSE(protoLayer.min(binX), -16.26345596_mm, 1e-4); + BOOST_CHECK_CLOSE(protoLayer.max(binX), 16.26345596_mm, 1e-4); + BOOST_CHECK_CLOSE(protoLayer.min(binY), -23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binY), 23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binZ), -16.26345596_mm, 1e-4); + BOOST_CHECK_CLOSE(protoLayer.max(binZ), 16.26345596_mm, 1e-4); + + protoLayer = {tgContext, surfaces, + Transform3{AngleAxis3{45_degree, Vector3::UnitY()}}.inverse()}; + + BOOST_CHECK_EQUAL(protoLayer.surfaces().size(), 8); + BOOST_CHECK_CLOSE(protoLayer.range(binX), 46_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binX), -23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binX), 23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.range(binY), 46_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binY), -23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binY), 23_mm, 1e-8); + CHECK_SMALL(protoLayer.range(binZ), 1e-14); + CHECK_SMALL(protoLayer.min(binZ), 1e-14); + CHECK_SMALL(protoLayer.max(binZ), 1e-14); + + surfaces = makeFan(0_degree, 10_mm); + + protoLayer = {tgContext, surfaces}; + + BOOST_CHECK_EQUAL(protoLayer.surfaces().size(), 8); + BOOST_CHECK_CLOSE(protoLayer.range(binX), 46_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binX), -23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binX), 23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.range(binY), 46_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binY), -23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binY), 23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.range(binZ), 10_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binZ), -5_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binZ), 5_mm, 1e-8); + + surfaces = makeFan(45_degree, 10_mm); + + protoLayer = {tgContext, surfaces, + Transform3{AngleAxis3{45_degree, Vector3::UnitY()}}.inverse()}; + + BOOST_CHECK_EQUAL(protoLayer.surfaces().size(), 8); + BOOST_CHECK_CLOSE(protoLayer.range(binX), 46_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binX), -23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binX), 23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.range(binY), 46_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binY), -23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binY), 23_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.range(binZ), 10_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.min(binZ), -5_mm, 1e-8); + BOOST_CHECK_CLOSE(protoLayer.max(binZ), 5_mm, 1e-8); +} + BOOST_AUTO_TEST_SUITE_END() } // namespace Acts::Test::Layers From 550c0d6cb82ec82e37f65f236ba099606c8e9619 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Tue, 8 Oct 2024 11:12:18 +0200 Subject: [PATCH 08/18] build: Use correct GM variables in cmake config (#3699) The generated CMake config was using the wrong version variables from GeoModel. --- cmake/ActsConfig.cmake.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/ActsConfig.cmake.in b/cmake/ActsConfig.cmake.in index b66c5ae3544..7ed4b3884a5 100644 --- a/cmake/ActsConfig.cmake.in +++ b/cmake/ActsConfig.cmake.in @@ -77,8 +77,8 @@ if(PluginPodio IN_LIST Acts_COMPONENTS) find_dependency(podio @podio_VERSION@ CONFIG EXACT) endif() if(PluginGeoModel IN_LIST Acts_COMPONENTS) - find_dependency(GeoModelCore @GeoModel_VERSION@ CONFIG EXACT) - find_dependency(GeoModelIO @GeoModel_VERSION@ CONFIG EXACT) + find_dependency(GeoModelCore @GeoModelCore_VERSION@ CONFIG EXACT) + find_dependency(GeoModelIO @GeoModelIO_VERSION@ CONFIG EXACT) endif() if (PluginHashing IN_LIST Acts_COMPONENTS) find_dependency(Annoy @ANNOY_VERSION@ CONFIG EXACT) From 198708bfb9b9bce7c53caf03ac5785910b8bb818 Mon Sep 17 00:00:00 2001 From: Benjamin Huth <37871400+benjaminhuth@users.noreply.github.com> Date: Tue, 8 Oct 2024 12:25:58 +0200 Subject: [PATCH 09/18] feat: GeoModel changes for Gen1 ITk (#3685) --- Examples/Detectors/CMakeLists.txt | 1 + .../ITkModuleSplitting/CMakeLists.txt | 7 + .../ITkModuleSplitting/ITkModuleSplitting.hpp | 147 ++++++++++++++++++ .../Detectors/TGeoDetector/CMakeLists.txt | 1 + .../src/TGeoITkModuleSplitter.cpp | 111 ++----------- Examples/Python/src/GeoModel.cpp | 66 +++++++- Plugins/GeoModel/CMakeLists.txt | 1 + .../GeoModel/GeoModelDetectorElement.hpp | 16 +- .../GeoModel/GeoModelDetectorElementITk.hpp | 87 +++++++++++ .../GeoModel/src/GeoModelDetectorElement.cpp | 6 + .../src/GeoModelDetectorElementITk.cpp | 131 ++++++++++++++++ .../src/GeoModelDetectorObjectFactory.cpp | 7 + .../UnitTests/Plugins/GeoModel/CMakeLists.txt | 1 + .../GeoModelDetectorElementITkTests.cpp | 90 +++++++++++ 14 files changed, 574 insertions(+), 98 deletions(-) create mode 100644 Examples/Detectors/ITkModuleSplitting/CMakeLists.txt create mode 100644 Examples/Detectors/ITkModuleSplitting/include/ActsExamples/ITkModuleSplitting/ITkModuleSplitting.hpp create mode 100644 Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElementITk.hpp create mode 100644 Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp create mode 100644 Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementITkTests.cpp diff --git a/Examples/Detectors/CMakeLists.txt b/Examples/Detectors/CMakeLists.txt index a04112d572c..7e7aed5daed 100644 --- a/Examples/Detectors/CMakeLists.txt +++ b/Examples/Detectors/CMakeLists.txt @@ -4,5 +4,6 @@ add_subdirectory(GenericDetector) add_subdirectory_if(Geant4Detector ACTS_BUILD_EXAMPLES_GEANT4) add_subdirectory(MagneticField) add_subdirectory(TGeoDetector) +add_subdirectory(ITkModuleSplitting) add_subdirectory(TelescopeDetector) add_subdirectory_if(MuonSpectrometerMockupDetector ACTS_BUILD_EXAMPLES_GEANT4) diff --git a/Examples/Detectors/ITkModuleSplitting/CMakeLists.txt b/Examples/Detectors/ITkModuleSplitting/CMakeLists.txt new file mode 100644 index 00000000000..affd7146336 --- /dev/null +++ b/Examples/Detectors/ITkModuleSplitting/CMakeLists.txt @@ -0,0 +1,7 @@ +add_library(ActsExamplesITkModuleSplitting INTERFACE) +target_include_directories( + ActsExamplesITkModuleSplitting + INTERFACE $ +) + +install(DIRECTORY include/ActsExamples DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) diff --git a/Examples/Detectors/ITkModuleSplitting/include/ActsExamples/ITkModuleSplitting/ITkModuleSplitting.hpp b/Examples/Detectors/ITkModuleSplitting/include/ActsExamples/ITkModuleSplitting/ITkModuleSplitting.hpp new file mode 100644 index 00000000000..591671bffb7 --- /dev/null +++ b/Examples/Detectors/ITkModuleSplitting/include/ActsExamples/ITkModuleSplitting/ITkModuleSplitting.hpp @@ -0,0 +1,147 @@ +// 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/. + +// This file is part of the Acts project. +// +// Copyright (C) 2024 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 http://mozilla.org/MPL/2.0/. + +#pragma once + +#include "Acts/Definitions/Algebra.hpp" +#include "Acts/Surfaces/AnnulusBounds.hpp" +#include "Acts/Surfaces/RectangleBounds.hpp" +#include "Acts/Surfaces/Surface.hpp" +#include "Acts/Surfaces/SurfaceBounds.hpp" + +#include +#include +#include +#include + +namespace ActsExamples::ITk { + +template +inline std::vector> splitBarrelModule( + const Acts::GeometryContext& gctx, + const std::shared_ptr& detElement, + unsigned int nSegments, const element_factory_t& factory, + const std::string& name, + const Acts::Logger& logger = Acts::getDummyLogger()) { + // Retrieve the surface + const Acts::Surface& surface = detElement->surface(); + const Acts::SurfaceBounds& bounds = surface.bounds(); + if (bounds.type() != Acts::SurfaceBounds::eRectangle || nSegments <= 1u) { + ACTS_WARNING("Invalid splitting config for barrel node: " + << name << "! Node will not be slpit."); + return {detElement}; + } + + // Output container for the submodules + std::vector> detElements = {}; + detElements.reserve(nSegments); + + // Get the geometric information + const Acts::Transform3& transform = surface.transform(gctx); + // Determine the new bounds + const std::vector boundsValues = bounds.values(); + + double lengthX = (boundsValues[Acts::RectangleBounds::eMaxX] - + boundsValues[Acts::RectangleBounds::eMinX]) / + nSegments; + double lengthY = boundsValues[Acts::RectangleBounds::eMaxY] - + boundsValues[Acts::RectangleBounds::eMinY]; + auto rectBounds = + std::make_shared(0.5 * lengthX, 0.5 * lengthY); + // Translation for every subelement + auto localTranslation = Acts::Vector2(-0.5 * lengthX * (nSegments - 1), 0.); + const auto step = Acts::Vector2(lengthX, 0.); + ACTS_DEBUG("Rectangle bounds for new node (half length): " + + std::to_string(rectBounds->halfLengthX()) + ", " + + std::to_string(rectBounds->halfLengthY())); + + for (std::size_t i = 0; i < nSegments; i++) { + Acts::Vector3 globalTranslation = + surface.localToGlobal(gctx, localTranslation, {}) - + transform.translation(); + auto elemTransform = + Acts::Transform3(transform).pretranslate(globalTranslation); + detElements.emplace_back(factory(elemTransform, rectBounds)); + + localTranslation += step; + } + return detElements; +} + +template +inline std::vector> splitDiscModule( + const Acts::GeometryContext& gctx, + const std::shared_ptr& detElement, + const std::vector>& splitRanges, + const element_factory_t& factory, const std::string& name, + const Acts::Logger& logger = Acts::getDummyLogger()) { + // Retrieve the surface + const Acts::Surface& surface = detElement->surface(); + const Acts::SurfaceBounds& bounds = surface.bounds(); + + // Check annulus bounds origin + auto printOrigin = [&](const Acts::Surface& sf) { + Acts::Vector3 discOrigin = + sf.localToGlobal(gctx, Acts::Vector2(0., 0.), Acts::Vector3::Zero()); + std::string out = + "Disc surface origin at: " + std::to_string(discOrigin[0]) + ", " + + std::to_string(discOrigin[1]) + ", " + std::to_string(discOrigin[2]); + return out; + }; + ACTS_DEBUG(printOrigin(surface)); + + if (bounds.type() != Acts::SurfaceBounds::eAnnulus || splitRanges.empty()) { + ACTS_WARNING("Invalid splitting config for disk node: " + << name << "! Node will not be slpit."); + return {detElement}; + } + + auto nSegments = splitRanges.size(); + + // Output container for the submodules + std::vector> detElements = {}; + detElements.reserve(nSegments); + + // Get the geometric information + const Acts::Transform3& transform = surface.transform(gctx); + const std::vector boundsValues = bounds.values(); + std::array values{}; + + std::copy_n(boundsValues.begin(), Acts::AnnulusBounds::eSize, values.begin()); + + for (std::size_t i = 0; i < nSegments; i++) { + if (boundsValues[Acts::AnnulusBounds::eMinR] > splitRanges[i].first || + boundsValues[Acts::AnnulusBounds::eMaxR] < splitRanges[i].second) { + ACTS_WARNING( + "Radius pattern not within the original bounds, node will not be " + "split!"); + return {detElement}; + } + + values[Acts::AnnulusBounds::eMinR] = splitRanges[i].first; + values[Acts::AnnulusBounds::eMaxR] = splitRanges[i].second; + auto annulusBounds = std::make_shared(values); + ACTS_DEBUG( + "New r bounds for node: " + std::to_string(annulusBounds->rMin()) + + ", " + std::to_string(annulusBounds->rMax())); + + auto element = factory(transform, annulusBounds); + detElements.push_back(std::move(element)); + } + return detElements; +} + +} // namespace ActsExamples::ITk diff --git a/Examples/Detectors/TGeoDetector/CMakeLists.txt b/Examples/Detectors/TGeoDetector/CMakeLists.txt index 04d065adaa5..3df36c62f15 100644 --- a/Examples/Detectors/TGeoDetector/CMakeLists.txt +++ b/Examples/Detectors/TGeoDetector/CMakeLists.txt @@ -17,6 +17,7 @@ target_link_libraries( ActsPluginJson ActsExamplesFramework ActsExamplesDetectorGeneric + ActsExamplesITkModuleSplitting ) install( diff --git a/Examples/Detectors/TGeoDetector/src/TGeoITkModuleSplitter.cpp b/Examples/Detectors/TGeoDetector/src/TGeoITkModuleSplitter.cpp index 0aafbe2d7d7..42bbbd9c882 100644 --- a/Examples/Detectors/TGeoDetector/src/TGeoITkModuleSplitter.cpp +++ b/Examples/Detectors/TGeoDetector/src/TGeoITkModuleSplitter.cpp @@ -14,6 +14,7 @@ #include "Acts/Surfaces/RectangleBounds.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Surfaces/SurfaceBounds.hpp" +#include "ActsExamples/ITkModuleSplitting/ITkModuleSplitting.hpp" #include #include @@ -86,55 +87,16 @@ ActsExamples::TGeoITkModuleSplitter::splitBarrelModule( const Acts::GeometryContext& gctx, const std::shared_ptr& detElement, unsigned int nSegments) const { - // Retrieve the surface - auto identifier = detElement->identifier(); - const Acts::Surface& surface = detElement->surface(); - const Acts::SurfaceBounds& bounds = surface.bounds(); - if (bounds.type() != Acts::SurfaceBounds::eRectangle || nSegments <= 1u) { - ACTS_WARNING("Invalid splitting config for barrel node: " + - std::string(detElement->tgeoNode().GetName()) + - "! Node will not be slpit."); - return {detElement}; - } - - // Output container for the submodules - std::vector> detElements = - {}; - detElements.reserve(nSegments); - - // Get the geometric information - double thickness = detElement->thickness(); - const Acts::Transform3& transform = surface.transform(gctx); - // Determine the new bounds - const std::vector boundsValues = bounds.values(); - double lengthX = (boundsValues[Acts::RectangleBounds::eMaxX] - - boundsValues[Acts::RectangleBounds::eMinX]) / - nSegments; - double lengthY = boundsValues[Acts::RectangleBounds::eMaxY] - - boundsValues[Acts::RectangleBounds::eMinY]; - auto rectBounds = - std::make_shared(0.5 * lengthX, 0.5 * lengthY); - // Translation for every subelement - auto localTranslation = Acts::Vector2(-0.5 * lengthX * (nSegments - 1), 0.); - const auto step = Acts::Vector2(lengthX, 0.); - ACTS_DEBUG("Rectangle bounds for new node (half length): " + - std::to_string(rectBounds->halfLengthX()) + ", " + - std::to_string(rectBounds->halfLengthY())); + auto name = detElement->tgeoNode().GetName(); - for (std::size_t i = 0; i < nSegments; i++) { - Acts::Vector3 globalTranslation = - surface.localToGlobal(gctx, localTranslation, {}) - - transform.translation(); - auto elemTransform = - Acts::Transform3(transform).pretranslate(globalTranslation); - auto element = std::make_shared( - identifier, detElement->tgeoNode(), elemTransform, rectBounds, - thickness); - detElements.push_back(std::move(element)); + auto factory = [&](const auto& trafo, const auto& bounds) { + return std::make_shared( + detElement->identifier(), detElement->tgeoNode(), trafo, bounds, + detElement->thickness()); + }; - localTranslation += step; - } - return detElements; + return ITk::splitBarrelModule(gctx, detElement, nSegments, factory, name, + logger()); } /// If applicable, returns a split detector element @@ -144,55 +106,14 @@ ActsExamples::TGeoITkModuleSplitter::splitDiscModule( const std::shared_ptr& detElement, const std::vector& splitRanges) const { - // Retrieve the surface - auto identifier = detElement->identifier(); - const Acts::Surface& surface = detElement->surface(); - const Acts::SurfaceBounds& bounds = surface.bounds(); + auto name = detElement->tgeoNode().GetName(); - // Check annulus bounds origin - auto printOrigin = [&](const Acts::Surface& sf) { - Acts::Vector3 discOrigin = - sf.localToGlobal(gctx, Acts::Vector2(0., 0.), Acts::Vector3::Zero()); - std::string out = - "Disc surface origin at: " + std::to_string(discOrigin[0]) + ", " + - std::to_string(discOrigin[1]) + ", " + std::to_string(discOrigin[2]); - return out; + auto factory = [&](const auto& trafo, const auto& bounds) { + return std::make_shared( + detElement->identifier(), detElement->tgeoNode(), trafo, bounds, + detElement->thickness()); }; - ACTS_DEBUG(printOrigin(surface)); - - if (bounds.type() != Acts::SurfaceBounds::eAnnulus || splitRanges.empty()) { - ACTS_WARNING("Invalid splitting config for disk node: " + - std::string(detElement->tgeoNode().GetName()) + - "! Node will not be slpit."); - return {detElement}; - } - - auto nSegments = splitRanges.size(); - - // Output container for the submodules - std::vector> detElements = - {}; - detElements.reserve(nSegments); - // Get the geometric information - double thickness = detElement->thickness(); - const Acts::Transform3& transform = surface.transform(gctx); - const std::vector boundsValues = bounds.values(); - std::array values{}; - std::copy_n(boundsValues.begin(), Acts::AnnulusBounds::eSize, values.begin()); - - for (std::size_t i = 0; i < nSegments; i++) { - values[Acts::AnnulusBounds::eMinR] = splitRanges[i].first; - values[Acts::AnnulusBounds::eMaxR] = splitRanges[i].second; - auto annulusBounds = std::make_shared(values); - ACTS_DEBUG( - "New r bounds for node: " + std::to_string(annulusBounds->rMin()) + - ", " + std::to_string(annulusBounds->rMax())); - - auto element = std::make_shared( - identifier, detElement->tgeoNode(), transform, annulusBounds, - thickness); - detElements.push_back(std::move(element)); - } - return detElements; + return ITk::splitDiscModule(gctx, detElement, splitRanges, factory, name, + logger()); } diff --git a/Examples/Python/src/GeoModel.cpp b/Examples/Python/src/GeoModel.cpp index 162062aa970..eb9b4406808 100644 --- a/Examples/Python/src/GeoModel.cpp +++ b/Examples/Python/src/GeoModel.cpp @@ -17,12 +17,17 @@ #include "Acts/Plugins/GeoModel/GeoModelBlueprintCreater.hpp" #include "Acts/Plugins/GeoModel/GeoModelConverters.hpp" #include "Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp" +#include "Acts/Plugins/GeoModel/GeoModelDetectorElementITk.hpp" #include "Acts/Plugins/GeoModel/GeoModelDetectorObjectFactory.hpp" #include "Acts/Plugins/GeoModel/GeoModelReader.hpp" #include "Acts/Plugins/GeoModel/GeoModelTree.hpp" #include "Acts/Plugins/GeoModel/IGeoShapeConverter.hpp" #include "Acts/Plugins/Python/Utilities.hpp" -#include "Acts/Surfaces/Surface.hpp" +#include "Acts/Surfaces/AnnulusBounds.hpp" +#include "Acts/Surfaces/DiscSurface.hpp" +#include "Acts/Surfaces/PlaneSurface.hpp" +#include "Acts/Surfaces/RectangleBounds.hpp" +#include "ActsExamples/ITkModuleSplitting/ITkModuleSplitting.hpp" #include @@ -46,7 +51,13 @@ void addGeoModel(Context& ctx) { py::class_>( - gm, "GeoModelDetectorElement"); + gm, "GeoModelDetectorElement") + .def("logVolName", &Acts::GeoModelDetectorElement::logVolName) + .def("databaseEntryName", + &Acts::GeoModelDetectorElement::databaseEntryName) + .def("surface", [](Acts::GeoModelDetectorElement self) { + return self.surface().getSharedPtr(); + }); // Shape converters { @@ -184,5 +195,56 @@ void addGeoModel(Context& ctx) { .def_readwrite("table", &Acts::GeoModelBlueprintCreater::Options::table); } + + gm.def( + "splitBarrelModule", + [](const Acts::GeometryContext& gctx, + std::shared_ptr detElement, + unsigned nSegments, Acts::Logging::Level logLevel) { + auto logger = Acts::getDefaultLogger("ITkSlitBarrel", logLevel); + auto name = detElement->databaseEntryName(); + + auto factory = [&](const auto& trafo, const auto& bounds) { + return Acts::GeoModelDetectorElement::createDetectorElement< + Acts::PlaneSurface, Acts::RectangleBounds>( + detElement->physicalVolume(), bounds, trafo, + detElement->thickness()); + }; + + return ActsExamples::ITk::splitBarrelModule(gctx, detElement, nSegments, + factory, name, *logger); + }, + "gxtx"_a, "detElement"_a, "nSegments"_a, + "logLevel"_a = Acts::Logging::INFO); + + gm.def( + "splitDiscModule", + [](const Acts::GeometryContext& gctx, + std::shared_ptr detElement, + const std::vector>& patterns, + Acts::Logging::Level logLevel) { + auto logger = Acts::getDefaultLogger("ITkSlitBarrel", logLevel); + auto name = detElement->databaseEntryName(); + + auto factory = [&](const auto& trafo, const auto& bounds) { + return Acts::GeoModelDetectorElement::createDetectorElement< + Acts::DiscSurface, Acts::AnnulusBounds>( + detElement->physicalVolume(), bounds, trafo, + detElement->thickness()); + }; + + return ActsExamples::ITk::splitDiscModule(gctx, detElement, patterns, + factory, name, *logger); + }, + "gxtx"_a, "detElement"_a, "splitRanges"_a, + "logLevel"_a = Acts::Logging::INFO); + + py::class_>( + gm, "GeoModelDetectorElementITk") + .def("surface", [](Acts::GeoModelDetectorElementITk& self) { + return self.surface().getSharedPtr(); + }); + gm.def("convertToItk", &GeoModelDetectorElementITk::convertFromGeomodel); } } // namespace Acts::Python diff --git a/Plugins/GeoModel/CMakeLists.txt b/Plugins/GeoModel/CMakeLists.txt index 05c4c9e8148..90a672a012b 100644 --- a/Plugins/GeoModel/CMakeLists.txt +++ b/Plugins/GeoModel/CMakeLists.txt @@ -20,6 +20,7 @@ add_library( src/detail/GeoModelBinningHelper.cpp src/detail/GeoModelExtentHelper.cpp src/detail/GeoUnionDoubleTrdConverter.cpp + src/GeoModelDetectorElementITk.cpp ) target_include_directories( ActsPluginGeoModel diff --git a/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp b/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp index eed4d2f62a3..ab1cb025a4a 100644 --- a/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp +++ b/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp @@ -87,7 +87,18 @@ class GeoModelDetectorElement : public DetectorElementBase { /// @return to the Geant4 physical volume PVConstLink physicalVolume() const; - private: + /// Get the name of the logical volume + const std::string& logVolName() const; + + /// Get the string identifier of the corresponding database entry + /// Note: This is not by defnitition a unique identifier, there can be + /// several detector elements created from a single database entry. + const std::string& databaseEntryName() const { return m_entryName; }; + + /// Set the corresponding database entry string + void setDatabaseEntryName(const std::string& n) { m_entryName = n; }; + + protected: /// Attach a surface /// /// @param surface The surface to attach @@ -95,6 +106,9 @@ class GeoModelDetectorElement : public DetectorElementBase { m_surface = std::move(surface); } + private: + std::string m_entryName; + /// The GeoModel full physical volume PVConstLink m_geoPhysVol{nullptr}; /// The surface diff --git a/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElementITk.hpp b/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElementITk.hpp new file mode 100644 index 00000000000..31cce3360e9 --- /dev/null +++ b/Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElementITk.hpp @@ -0,0 +1,87 @@ +// 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/. + +// This file is part of the Acts project. +// +// Copyright (C) 2024 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 http://mozilla.org/MPL/2.0/. + +#pragma once + +#include "Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp" +#include "Acts/Utilities/MultiIndex.hpp" + +namespace Acts { + +class ITkIdentifier { + Acts::MultiIndex m_identifier{}; + + public: + ITkIdentifier(int hardware, int barrelEndcap, int layerWheel, int etaModule, + int phiModule, int side); + + /// Access the hardware specifier (pixel=0, strip=1) + int hardware() const; + + /// Access the barrel-endcap specifier (-2,0,2) + int barrelEndcap() const; + + /// Access the layer specifier + int layerWheel() const; + + /// Access the phi module specifier + int phiModule() const; + + /// Access the eta module specifier + int etaModule() const; + + /// Access the side (for double sided strip modules) + int side() const; + + /// A unique identifier that represents the combination of specifiers + std::size_t value() const; +}; + +std::ostream& operator<<(std::ostream& os, const ITkIdentifier& id); + +/// Specialization of the GeoModelDetectorElement for the ITk. This allows +/// mapping of Acts::GeometryIdentifiers to ITk modules in a straight-forward +/// way. +class GeoModelDetectorElementITk : public GeoModelDetectorElement { + public: + GeoModelDetectorElementITk(const PVConstLink& geoPhysVol, + std::shared_ptr surface, + const Transform3& sfTransform, + ActsScalar thickness, int hardware, + int barrelEndcap, int layerWheel, int etaModule, + int phiModule, int side) + : GeoModelDetectorElement(geoPhysVol, std::move(surface), sfTransform, + thickness), + m_identifier(hardware, barrelEndcap, layerWheel, etaModule, phiModule, + side) {} + + ITkIdentifier identifier() const { return m_identifier; } + + /// Convert a GeoModelDetectorElement to a GeoModelDetectorElementITk + /// A new surface is constructed. + /// @todo Remove redundancy in signature once plugin is refactored + static std::tuple, + std::shared_ptr> + convertFromGeomodel(std::shared_ptr detEl, + std::shared_ptr srf, const GeometryContext& gctx, + int hardware, int barrelEndcap, int layerWheel, + int etaModule, int phiModule, int side); + + private: + ITkIdentifier m_identifier; +}; + +} // namespace Acts diff --git a/Plugins/GeoModel/src/GeoModelDetectorElement.cpp b/Plugins/GeoModel/src/GeoModelDetectorElement.cpp index 81526fc7693..279a7b802b5 100644 --- a/Plugins/GeoModel/src/GeoModelDetectorElement.cpp +++ b/Plugins/GeoModel/src/GeoModelDetectorElement.cpp @@ -12,6 +12,8 @@ #include +#include + Acts::GeoModelDetectorElement::GeoModelDetectorElement( PVConstLink geoPhysVol, std::shared_ptr surface, const Transform3& sfTransform, ActsScalar thickness) @@ -40,3 +42,7 @@ Acts::ActsScalar Acts::GeoModelDetectorElement::thickness() const { PVConstLink Acts::GeoModelDetectorElement::physicalVolume() const { return m_geoPhysVol; } + +const std::string& Acts::GeoModelDetectorElement::logVolName() const { + return m_geoPhysVol->getLogVol()->getName(); +} diff --git a/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp b/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp new file mode 100644 index 00000000000..f671386dec6 --- /dev/null +++ b/Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp @@ -0,0 +1,131 @@ +// 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/. + +// This file is part of the Acts project. +// +// Copyright (C) 2024 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 http://mozilla.org/MPL/2.0/. + +#include "Acts/Plugins/GeoModel/GeoModelDetectorElementITk.hpp" + +#include "Acts/Surfaces/AnnulusBounds.hpp" +#include "Acts/Surfaces/DiscSurface.hpp" +#include "Acts/Surfaces/PlaneSurface.hpp" +#include "Acts/Surfaces/RectangleBounds.hpp" + +#include + +namespace Acts { + +// Mapping between the barrel-endcap identifier and its unsigned representation +constexpr static std::array, 3> s_barrelEndcapMap{ + {{0, 0}, {1, 2}, {2, -2}}}; + +Acts::ITkIdentifier::ITkIdentifier(int hardware, int barrelEndcap, + int layerWheel, int etaModule, int phiModule, + int side) { + assert((hardware == 0) || (hardware == 1)); + assert((barrelEndcap == 2) || (barrelEndcap == -2) || (barrelEndcap == 0)); + assert(layerWheel >= 0); + assert(phiModule >= 0); + assert((side == 0) || (side == 1)); + + m_identifier.set(0, hardware); + + auto found = std::ranges::find(s_barrelEndcapMap, barrelEndcap, + &std::pair::second); + if (found == s_barrelEndcapMap.end()) { + throw std::invalid_argument("Invalid barrel-endcap specifier"); + } + m_identifier.set(1, found->first); + m_identifier.set(2, layerWheel); + m_identifier.set(3, static_cast(etaModule < 0)); + m_identifier.set(4, std::abs(etaModule)); + m_identifier.set(5, phiModule); + m_identifier.set(6, side); +} + +int Acts::ITkIdentifier::hardware() const { + return m_identifier.level(0); +} + +int Acts::ITkIdentifier::barrelEndcap() const { + auto found = std::ranges::find(s_barrelEndcapMap, m_identifier.level(1), + &std::pair::first); + if (found == s_barrelEndcapMap.end()) { + throw std::invalid_argument("Invalid barrel-endcap specifier"); + } + return found->second; +} + +int Acts::ITkIdentifier::layerWheel() const { + return m_identifier.level(2); +} + +int Acts::ITkIdentifier::etaModule() const { + int sign = (m_identifier.level(3) == 0) ? 1 : -1; + return sign * m_identifier.level(4); +} + +int Acts::ITkIdentifier::phiModule() const { + return m_identifier.level(5); +} + +int Acts::ITkIdentifier::side() const { + return m_identifier.level(6); +} + +std::size_t Acts::ITkIdentifier::value() const { + return m_identifier.value(); +} + +std::ostream &operator<<(std::ostream &os, const ITkIdentifier &id) { + os << "(hw: " << id.hardware() << ", be: " << id.barrelEndcap() + << ", lw: " << id.layerWheel() << ", em: " << id.etaModule() + << ", pm: " << id.phiModule() << ", sd: " << id.side() << ")"; + return os; +} + +std::tuple, + std::shared_ptr> +Acts::GeoModelDetectorElementITk::convertFromGeomodel( + std::shared_ptr detEl, + std::shared_ptr srf, const GeometryContext &gctx, int hardware, + int barrelEndcap, int layerWheel, int etaModule, int phiModule, int side) { + auto helper = [&]() { + auto bounds = std::make_shared( + dynamic_cast(srf->bounds())); + + auto itkEl = std::make_shared( + detEl->physicalVolume(), nullptr, detEl->transform(gctx), + detEl->thickness(), hardware, barrelEndcap, layerWheel, etaModule, + phiModule, side); + auto surface = Surface::makeShared(bounds, *itkEl.get()); + + itkEl->attachSurface(surface); + itkEl->setDatabaseEntryName(detEl->databaseEntryName()); + return std::pair{itkEl, surface}; + }; + + if (srf->type() == Acts::Surface::Plane && + srf->bounds().type() == Acts::SurfaceBounds::eRectangle) { + return helper.operator()(); + } + if (srf->type() == Acts::Surface::Disc && + srf->bounds().type() == Acts::SurfaceBounds::eAnnulus) { + return helper.operator()(); + } + + throw std::runtime_error( + "Only Plane+Rectangle and Disc+Annulus are converted for the ITk"); +} + +} // namespace Acts diff --git a/Plugins/GeoModel/src/GeoModelDetectorObjectFactory.cpp b/Plugins/GeoModel/src/GeoModelDetectorObjectFactory.cpp index a8844d9fa5d..b68bb585358 100644 --- a/Plugins/GeoModel/src/GeoModelDetectorObjectFactory.cpp +++ b/Plugins/GeoModel/src/GeoModelDetectorObjectFactory.cpp @@ -151,6 +151,7 @@ bool Acts::GeoModelDetectorObjectFactory::convertBox(std::string name) { void Acts::GeoModelDetectorObjectFactory::convertFpv( const std::string &name, GeoFullPhysVol *fpv, Cache &cache, const GeometryContext &gctx) { + const auto prevSize = cache.sensitiveSurfaces.size(); PVConstLink physVol{fpv}; // get children @@ -185,6 +186,12 @@ void Acts::GeoModelDetectorObjectFactory::convertFpv( const Transform3 &transform = fpv->getAbsoluteTransform(); convertSensitive(fpv, transform, cache.sensitiveSurfaces); } + + // Set the corresponding database entry name to all sensitive surfaces + for (auto i = prevSize; i < cache.sensitiveSurfaces.size(); ++i) { + auto &[detEl, _] = cache.sensitiveSurfaces[i]; + detEl->setDatabaseEntryName(name); + } } // function to determine if object fits query bool Acts::GeoModelDetectorObjectFactory::matches(const std::string &name, diff --git a/Tests/UnitTests/Plugins/GeoModel/CMakeLists.txt b/Tests/UnitTests/Plugins/GeoModel/CMakeLists.txt index 61711992918..a09476dae5b 100644 --- a/Tests/UnitTests/Plugins/GeoModel/CMakeLists.txt +++ b/Tests/UnitTests/Plugins/GeoModel/CMakeLists.txt @@ -4,6 +4,7 @@ add_unittest(GeoModelDetectorElement GeoModelDetectorElementTests.cpp) add_unittest(GeoBoxConverter GeoBoxConverterTests.cpp) add_unittest(GeoTrdConverter GeoTrdConverterTests.cpp) add_unittest(GeoDetectorObjectTest GeoDetectorObjectTest.cpp) +add_unittest(GeoModelDetectorElementITk GeoModelDetectorElementITkTests.cpp) add_unittest(GeoPolyConverterTests GeoPolyConverterTests.cpp) add_unittest(GeoTrdToVolumeTest GeoTrdToVolumeTests.cpp) add_unittest(GeoBoxToVolumeTest GeoBoxToVolumeTest.cpp) diff --git a/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementITkTests.cpp b/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementITkTests.cpp new file mode 100644 index 00000000000..0c945ebb6dd --- /dev/null +++ b/Tests/UnitTests/Plugins/GeoModel/GeoModelDetectorElementITkTests.cpp @@ -0,0 +1,90 @@ +// 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/. + +// This file is part of the Acts project. +// +// Copyright (C) 2024 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 http://mozilla.org/MPL/2.0/. + +#include + +#include "Acts/Plugins/GeoModel/GeoModelDetectorElementITk.hpp" +#include "Acts/Surfaces/PlaneSurface.hpp" +#include "Acts/Surfaces/RectangleBounds.hpp" +#include "Acts/Surfaces/Surface.hpp" + +#include +#include +#include +#include + +BOOST_AUTO_TEST_SUITE(GeoModelPlugin) + +BOOST_AUTO_TEST_CASE(ITkIdentifierTests) { + auto test = [](int hw, int bec, int lw, int em, int pm, int side) { + Acts::ITkIdentifier id(hw, bec, lw, em, pm, side); + BOOST_CHECK_EQUAL(id.hardware(), hw); + BOOST_CHECK_EQUAL(id.barrelEndcap(), bec); + BOOST_CHECK_EQUAL(id.layerWheel(), lw); + BOOST_CHECK_EQUAL(id.etaModule(), em); + BOOST_CHECK_EQUAL(id.phiModule(), pm); + BOOST_CHECK_EQUAL(id.side(), side); + }; + + for (int hw : {0, 1}) { + for (int bec : {-2, 0, 2}) { + for (int lw : {0, 10}) { + for (int em : {-10, 0, 10}) { + for (int pm : {10, 0}) { + for (int side : {0, 1}) { + test(hw, bec, lw, em, pm, side); + } + } + } + } + } + } +} + +BOOST_AUTO_TEST_CASE(GeoModelDetectorElementConstruction) { + Acts::GeometryContext gctx{}; + + auto material = GeoIntrusivePtr(new GeoMaterial("Material", 1.0)); + auto box = GeoIntrusivePtr(new GeoBox(100, 200, 2)); + auto log = GeoIntrusivePtr(new GeoLogVol("LogVolumeXY", box, material)); + auto fphys = GeoIntrusivePtr(new GeoFullPhysVol(log)); + auto rBounds = std::make_shared(100, 200); + + auto element = + Acts::GeoModelDetectorElement::createDetectorElement( + fphys, rBounds, Acts::Transform3::Identity(), 2.0); + + const int hardware = 0, barrelEndcap = -2, layerWheel = 100, phiModule = 200, + etaModule = 300, side = 1; + + auto [itkElement, _] = Acts::GeoModelDetectorElementITk::convertFromGeomodel( + element, element->surface().getSharedPtr(), gctx, hardware, barrelEndcap, + layerWheel, etaModule, phiModule, side); + + BOOST_CHECK_EQUAL(element->surface().type(), itkElement->surface().type()); + BOOST_CHECK_EQUAL(element->surface().bounds().type(), + itkElement->surface().bounds().type()); + BOOST_CHECK_NE(element->surface().associatedDetectorElement(), + itkElement->surface().associatedDetectorElement()); + BOOST_CHECK_EQUAL(itkElement->identifier().barrelEndcap(), barrelEndcap); + BOOST_CHECK_EQUAL(itkElement->identifier().hardware(), hardware); + BOOST_CHECK_EQUAL(itkElement->identifier().layerWheel(), layerWheel); + BOOST_CHECK_EQUAL(itkElement->identifier().phiModule(), phiModule); + BOOST_CHECK_EQUAL(itkElement->identifier().etaModule(), etaModule); + BOOST_CHECK_EQUAL(itkElement->identifier().side(), side); +} + +BOOST_AUTO_TEST_SUITE_END() From 69e0b6b181a14daab9b99e1ae6b576914a060afb Mon Sep 17 00:00:00 2001 From: Andreas Salzburger Date: Tue, 8 Oct 2024 15:01:32 +0200 Subject: [PATCH 10/18] feat: add some SVG glue code to help displaying proto material (#3692) This adds and little glue code to show proto material configurations: ![Screenshot 2024-10-04 at 13 42 02](https://github.com/user-attachments/assets/4e2adff2-ceda-4598-bec0-735e7b086129) This allows to display on mouse over which material mapping configuration was chosen. --- .../Acts/Material/ISurfaceMaterial.hpp | 10 +++++ .../ActsExamples/Io/Svg/SvgPointWriter.hpp | 4 +- Examples/Python/src/Geometry.cpp | 4 +- Examples/Python/src/Material.cpp | 17 ++++++- Examples/Python/src/Svg.cpp | 6 +++ .../include/Acts/Plugins/ActSVG/SvgUtils.hpp | 44 ++++++++++++------- Plugins/ActSVG/src/SurfaceSvgConverter.cpp | 15 ++----- 7 files changed, 68 insertions(+), 32 deletions(-) diff --git a/Core/include/Acts/Material/ISurfaceMaterial.hpp b/Core/include/Acts/Material/ISurfaceMaterial.hpp index d26563340cf..a379e18e715 100644 --- a/Core/include/Acts/Material/ISurfaceMaterial.hpp +++ b/Core/include/Acts/Material/ISurfaceMaterial.hpp @@ -15,6 +15,7 @@ #include "Acts/Material/MaterialSlab.hpp" #include +#include #include namespace Acts { @@ -117,6 +118,15 @@ class ISurfaceMaterial { /// Output Method for std::ostream, to be overloaded by child classes virtual std::ostream& toStream(std::ostream& sl) const = 0; + /// @brief output into a string + /// + /// @return the string representation + std::string toString() const { + std::stringstream sstrm; + toStream(sstrm); + return sstrm.str(); + } + protected: double m_splitFactor{1.}; //!< the split factor in favour of oppositePre MappingType m_mappingType{ diff --git a/Examples/Io/Svg/include/ActsExamples/Io/Svg/SvgPointWriter.hpp b/Examples/Io/Svg/include/ActsExamples/Io/Svg/SvgPointWriter.hpp index 632ce5efab0..3ad6527a882 100644 --- a/Examples/Io/Svg/include/ActsExamples/Io/Svg/SvgPointWriter.hpp +++ b/Examples/Io/Svg/include/ActsExamples/Io/Svg/SvgPointWriter.hpp @@ -87,6 +87,7 @@ class SvgPointWriter final : public WriterT> { s_pointStyle; //!< The style of the space point to be drawn std::string infoBoxTitle = ""; //!< If an info box title is set, draw it + Acts::Svg::Style infoTitleStyle = s_infoStyle; Acts::Svg::Style infoBoxStyle = s_infoStyle; // The style of the info box bool projectionXY = true; ///< xy projection @@ -170,7 +171,8 @@ ActsExamples::ProcessCode ActsExamples::SvgPointWriter::writeT( auto xyIbox = Acts::Svg::infoBox( static_cast(point3D.x() + 10.), static_cast(point3D.y() - 10.), m_cfg.infoBoxTitle, - {"Position: " + Acts::toString(point3D)}, m_cfg.infoBoxStyle, p); + m_cfg.infoTitleStyle, {"Position: " + Acts::toString(point3D)}, + m_cfg.infoBoxStyle, p); xyView.add_object(xyIbox); } } diff --git a/Examples/Python/src/Geometry.cpp b/Examples/Python/src/Geometry.cpp index 05484845465..8ec8ed97ef1 100644 --- a/Examples/Python/src/Geometry.cpp +++ b/Examples/Python/src/Geometry.cpp @@ -32,6 +32,7 @@ #include "Acts/Geometry/TrackingGeometry.hpp" #include "Acts/Geometry/Volume.hpp" #include "Acts/Geometry/VolumeBounds.hpp" +#include "Acts/Material/ISurfaceMaterial.hpp" #include "Acts/Plugins/Python/Utilities.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Surfaces/SurfaceArray.hpp" @@ -115,7 +116,8 @@ void addGeometry(Context& ctx) { [](const Surface& self) { return self.geometryId(); }) .def("center", &Surface::center) .def("type", &Surface::type) - .def("visualize", &Surface::visualize); + .def("visualize", &Surface::visualize) + .def("surfaceMaterial", &Acts::Surface::surfaceMaterialSharedPtr); } { diff --git a/Examples/Python/src/Material.cpp b/Examples/Python/src/Material.cpp index bbc7199e237..5bc5aa6a1f3 100644 --- a/Examples/Python/src/Material.cpp +++ b/Examples/Python/src/Material.cpp @@ -9,6 +9,7 @@ #include "Acts/Geometry/GeometryContext.hpp" #include "Acts/MagneticField/MagneticFieldContext.hpp" #include "Acts/Material/BinnedSurfaceMaterialAccumulater.hpp" +#include "Acts/Material/HomogeneousSurfaceMaterial.hpp" #include "Acts/Material/IMaterialDecorator.hpp" #include "Acts/Material/ISurfaceMaterial.hpp" #include "Acts/Material/IVolumeMaterial.hpp" @@ -16,6 +17,7 @@ #include "Acts/Material/MaterialMapper.hpp" #include "Acts/Material/MaterialValidater.hpp" #include "Acts/Material/PropagatorMaterialAssigner.hpp" +#include "Acts/Material/ProtoSurfaceMaterial.hpp" #include "Acts/Material/SurfaceMaterialMapper.hpp" #include "Acts/Material/VolumeMaterialMapper.hpp" #include "Acts/Plugins/Json/ActsJson.hpp" @@ -57,7 +59,20 @@ void addMaterial(Context& ctx) { { py::class_>( - m, "ISurfaceMaterial"); + m, "ISurfaceMaterial") + .def("toString", &Acts::ISurfaceMaterial::toString); + + py::class_>( + m, "ProtoGridSurfaceMaterial"); + + py::class_>(m, + "ProtoSurfaceMaterial"); + + py::class_>( + m, "HomogeneousSurfaceMaterial"); py::class_>( m, "IVolumeMaterial"); diff --git a/Examples/Python/src/Svg.cpp b/Examples/Python/src/Svg.cpp index ee011920947..301ceea1af3 100644 --- a/Examples/Python/src/Svg.cpp +++ b/Examples/Python/src/Svg.cpp @@ -225,6 +225,10 @@ void addSvg(Context& ctx) { ACTS_PYTHON_MEMBER(highlights); ACTS_PYTHON_MEMBER(strokeWidth); ACTS_PYTHON_MEMBER(strokeColor); + ACTS_PYTHON_MEMBER(highlightStrokeWidth); + ACTS_PYTHON_MEMBER(highlightStrokeColor); + ACTS_PYTHON_MEMBER(fontSize); + ACTS_PYTHON_MEMBER(fontColor); ACTS_PYTHON_MEMBER(quarterSegments); ACTS_PYTHON_STRUCT_END(); } @@ -297,6 +301,8 @@ void addSvg(Context& ctx) { svg.def("drawArrow", &actsvg::draw::arrow); svg.def("drawText", &actsvg::draw::text); + + svg.def("drawInfoBox", &Svg::infoBox); } // Draw Eta Lines diff --git a/Plugins/ActSVG/include/Acts/Plugins/ActSVG/SvgUtils.hpp b/Plugins/ActSVG/include/Acts/Plugins/ActSVG/SvgUtils.hpp index 0b36d7f82c9..38189871efb 100644 --- a/Plugins/ActSVG/include/Acts/Plugins/ActSVG/SvgUtils.hpp +++ b/Plugins/ActSVG/include/Acts/Plugins/ActSVG/SvgUtils.hpp @@ -38,6 +38,7 @@ struct Style { std::vector strokeDasharray = {}; unsigned int fontSize = 14u; + std::array fontColor = {0}; /// Number of segments to approximate a quarter of a circle unsigned int quarterSegments = 72u; @@ -60,6 +61,19 @@ struct Style { return std::tie(fll, str); } + + /// Conversion to fill, stroke and font + /// @return a tuple of actsvg digestable objects + std::tuple + fillStrokeFont() const { + auto [fll, str] = fillAndStroke(); + + actsvg::style::font fnt; + fnt._size = fontSize; + fnt._fc._rgb = fontColor; + + return std::tie(fll, str, fnt); + } }; /// Create a group @@ -133,31 +147,27 @@ inline static actsvg::svg::object axesXY(ActsScalar xMin, ActsScalar xMax, /// @param xPos the minimum x value /// @param yPos the maximum x value /// @param title the title of the info box +/// @param titleStyle the title of the info box /// @param info the text of the info box -/// @param infoBoxStyle the style of the info box +/// @param infoStyle the style of the info box (body) /// @param object the connected object /// /// @return an svg object -inline static actsvg::svg::object infoBox(ActsScalar xPos, ActsScalar yPos, - const std::string& title, - const std::vector& info, - const Style& infoBoxStyle, - const actsvg::svg::object& object) { - auto [fill, stroke] = infoBoxStyle.fillAndStroke(); - - actsvg::style::font titleFont; - titleFont._fc = actsvg::style::color{{255, 255, 255}}; - titleFont._size = infoBoxStyle.fontSize; - - actsvg::style::fill infoFill = fill; - infoFill._fc._opacity = 0.4; - actsvg::style::font infoFont; - infoFont._size = infoBoxStyle.fontSize; +inline static actsvg::svg::object infoBox( + ActsScalar xPos, ActsScalar yPos, const std::string& title, + const Style& titleStyle, const std::vector& info, + const Style& infoStyle, actsvg::svg::object& object, + const std::vector& highlights = {"mouseover", "mouseout"}) { + auto [titleFill, titleStroke, titleFont] = titleStyle.fillStrokeFont(); + auto [infoFill, infoStroke, infoFont] = infoStyle.fillStrokeFont(); + + actsvg::style::stroke stroke; return actsvg::draw::connected_info_box( object._id + "_infoBox", {static_cast(xPos), static_cast(yPos)}, - title, fill, titleFont, info, infoFill, infoFont, stroke, object); + title, titleFill, titleFont, info, infoFill, infoFont, stroke, object, + highlights); } /// Helper method to write to file diff --git a/Plugins/ActSVG/src/SurfaceSvgConverter.cpp b/Plugins/ActSVG/src/SurfaceSvgConverter.cpp index d9fb864a1f4..8cc7a78d2df 100644 --- a/Plugins/ActSVG/src/SurfaceSvgConverter.cpp +++ b/Plugins/ActSVG/src/SurfaceSvgConverter.cpp @@ -130,18 +130,9 @@ Acts::Svg::ProtoSurface Acts::Svg::SurfaceConverter::convert( geoId._id = std::to_string(surface.geometryId().value()); pSurface._decorations["geo_id"] = geoId; - // Attach the style - pSurface._fill._fc = { - cOptions.style.fillColor, - static_cast(cOptions.style.fillOpacity)}; - - // Fill style - pSurface._fill._fc._hl_rgb = cOptions.style.highlightColor; - pSurface._fill._fc._highlight = cOptions.style.highlights; - - // Stroke style - pSurface._stroke._sc = actsvg::style::color{cOptions.style.strokeColor}; - pSurface._stroke._width = cOptions.style.strokeWidth; + auto [surfaceFill, surfaceStroke] = cOptions.style.fillAndStroke(); + pSurface._fill = surfaceFill; + pSurface._stroke = surfaceStroke; return pSurface; } From e6400b6084c885cbf54c9e507483f92fdf6a32f1 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Wed, 9 Oct 2024 09:55:33 +0200 Subject: [PATCH 11/18] refactor: Remove quick math helpers (#3701) This has proven not to be useful as `std` math functions are not really a bottleneck in our current code. I propose to remove them here. If they become useful in the future we can just bring them back from here. --- Core/include/Acts/Propagator/EigenStepper.ipp | 9 +- Core/include/Acts/Utilities/QuickMath.hpp | 90 ------------------ Core/src/Propagator/SympyStepper.cpp | 9 +- Tests/Benchmarks/CMakeLists.txt | 1 - Tests/Benchmarks/QuickMathBenchmark.cpp | 92 ------------------- Tests/UnitTests/Core/Utilities/CMakeLists.txt | 1 - .../Core/Utilities/QuickMathTests.cpp | 64 ------------- 7 files changed, 2 insertions(+), 264 deletions(-) delete mode 100644 Core/include/Acts/Utilities/QuickMath.hpp delete mode 100644 Tests/Benchmarks/QuickMathBenchmark.cpp delete mode 100644 Tests/UnitTests/Core/Utilities/QuickMathTests.cpp diff --git a/Core/include/Acts/Propagator/EigenStepper.ipp b/Core/include/Acts/Propagator/EigenStepper.ipp index cd08613f722..84b305ae431 100644 --- a/Core/include/Acts/Propagator/EigenStepper.ipp +++ b/Core/include/Acts/Propagator/EigenStepper.ipp @@ -10,7 +10,6 @@ #include "Acts/Propagator/ConstrainedStep.hpp" #include "Acts/Propagator/EigenStepperError.hpp" #include "Acts/Propagator/detail/CovarianceEngine.hpp" -#include "Acts/Utilities/QuickMath.hpp" #include @@ -177,17 +176,11 @@ Acts::Result Acts::EigenStepper::step( // This is given by the order of the Runge-Kutta method constexpr double exponent = 0.25; - // Whether to use fast power function if available - constexpr bool tryUseFastPow{false}; - double x = state.options.stepping.stepTolerance / errorEstimate_; - if constexpr (exponent == 0.25 && !tryUseFastPow) { + if constexpr (exponent == 0.25) { // This is 3x faster than std::pow x = std::sqrt(std::sqrt(x)); - } else if constexpr (std::numeric_limits::is_iec559 && - tryUseFastPow) { - x = fastPow(x, exponent); } else { x = std::pow(x, exponent); } diff --git a/Core/include/Acts/Utilities/QuickMath.hpp b/Core/include/Acts/Utilities/QuickMath.hpp deleted file mode 100644 index 2f03ebb5e50..00000000000 --- a/Core/include/Acts/Utilities/QuickMath.hpp +++ /dev/null @@ -1,90 +0,0 @@ -// 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 -#include -#include -#include - -namespace Acts { - -/// @brief Fast inverse square root function -/// Taken from https://en.wikipedia.org/wiki/Fast_inverse_square_root -/// @param x the number to calculate the inverse square root of -/// @param iterations the number of newton iterations to perform -template -constexpr T fastInverseSqrt(T x, int iterations = 1) noexcept { - static_assert(std::numeric_limits::is_iec559 && - "Only IEEE 754 is supported"); - static_assert(std::is_same_v || std::is_same_v, - "Only float and double are supported"); - using I = std::conditional_t, std::uint32_t, - std::uint64_t>; - - constexpr I magic = - std::is_same_v ? 0x5f3759df : 0x5fe6eb50c7b537a9; - - union { - T f; - I i; - } u = {x}; - - u.i = magic - (u.i >> 1); - - for (int i = 0; i < iterations; ++i) { - u.f *= 1.5f - 0.5f * x * u.f * u.f; - } - - return u.f; -} - -/// @brief Fast power function @see `std::pow` -/// Taken from -/// https://martin.ankerl.com/2012/01/25/optimized-approximative-pow-in-c-and-cpp -/// @param a the base -/// @param b the exponent -constexpr double fastPow(double a, double b) { - // enable only on IEEE 754 - static_assert(std::numeric_limits::is_iec559); - - constexpr std::int64_t magic = 0x3FEF127F00000000; - - union { - double f; - std::int64_t i; - } u = {a}; - - u.i = static_cast(b * (u.i - magic) + magic); - - return u.f; -} - -/// @brief Fast power function more precise than @see `fastPow` -/// Taken from -/// https://martin.ankerl.com/2012/01/25/optimized-approximative-pow-in-c-and-cpp -/// @param a the base -/// @param b the exponent -constexpr double fastPowMorePrecise(double a, double b) { - // binary exponentiation - double r = 1.0; - int exp = std::abs(static_cast(b)); - double base = b > 0 ? a : 1 / a; - while (exp != 0) { - if ((exp & 1) != 0) { - r *= base; - } - base *= base; - exp >>= 1; - } - - return r * fastPow(a, b - static_cast(b)); -} - -} // namespace Acts diff --git a/Core/src/Propagator/SympyStepper.cpp b/Core/src/Propagator/SympyStepper.cpp index 167f44dd7f5..a33e1f567bb 100644 --- a/Core/src/Propagator/SympyStepper.cpp +++ b/Core/src/Propagator/SympyStepper.cpp @@ -10,7 +10,6 @@ #include "Acts/Propagator/detail/SympyCovarianceEngine.hpp" #include "Acts/Propagator/detail/SympyJacobianEngine.hpp" -#include "Acts/Utilities/QuickMath.hpp" #include #include @@ -127,17 +126,11 @@ Result SympyStepper::stepImpl( // This is given by the order of the Runge-Kutta method constexpr double exponent = 0.25; - // Whether to use fast power function if available - constexpr bool tryUseFastPow{false}; - double x = stepTolerance / errorEstimate_; - if constexpr (exponent == 0.25 && !tryUseFastPow) { + if constexpr (exponent == 0.25) { // This is 3x faster than std::pow x = std::sqrt(std::sqrt(x)); - } else if constexpr (std::numeric_limits::is_iec559 && - tryUseFastPow) { - x = fastPow(x, exponent); } else { x = std::pow(x, exponent); } diff --git a/Tests/Benchmarks/CMakeLists.txt b/Tests/Benchmarks/CMakeLists.txt index 8906375caea..b19e67fed59 100644 --- a/Tests/Benchmarks/CMakeLists.txt +++ b/Tests/Benchmarks/CMakeLists.txt @@ -29,7 +29,6 @@ add_benchmark(SurfaceIntersection SurfaceIntersectionBenchmark.cpp) add_benchmark(RayFrustum RayFrustumBenchmark.cpp) add_benchmark(AnnulusBounds AnnulusBoundsBenchmark.cpp) add_benchmark(StraightLineStepper StraightLineStepperBenchmark.cpp) -add_benchmark(QuickMath QuickMathBenchmark.cpp) add_benchmark(SympyStepper SympyStepperBenchmark.cpp) add_benchmark(Stepper StepperBenchmark.cpp) add_benchmark(SourceLink SourceLinkBenchmark.cpp) diff --git a/Tests/Benchmarks/QuickMathBenchmark.cpp b/Tests/Benchmarks/QuickMathBenchmark.cpp deleted file mode 100644 index 0cbec176b94..00000000000 --- a/Tests/Benchmarks/QuickMathBenchmark.cpp +++ /dev/null @@ -1,92 +0,0 @@ -// 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 -#include - -#include "Acts/Tests/CommonHelpers/BenchmarkTools.hpp" -#include "Acts/Utilities/QuickMath.hpp" - -#include -#include - -namespace bdata = boost::unit_test::data; - -namespace Acts::Test { - -/// @brief Another fast power function @see `fastPow` -// Taken from -// https://martin.ankerl.com/2007/02/11/optimized-exponential-functions-for-java -/// @param a the base -/// @param b the exponent -constexpr double fastPowAnother(double a, double b) { - // enable only on IEEE 754 - static_assert(std::numeric_limits::is_iec559); - - union { - double f; - std::int64_t i; - } u = {}; - - u.i = static_cast( - 9076650 * (a - 1) / (a + 1 + 4 * std::sqrt(a)) * b + 1072632447); - u.i <<= 32; - - // result seems broken? - return u.f; -} - -// Some randomness & number crunching -const unsigned int nTests = 10; -const unsigned int nReps = 10000; - -BOOST_DATA_TEST_CASE( - benchmark_pow_25, - bdata::random( - (bdata::engine = std::mt19937(), bdata::seed = 21, - bdata::distribution = std::uniform_real_distribution(-4, 4))) ^ - bdata::xrange(nTests), - baseExp, index) { - (void)index; - - const double base = std::pow(10, baseExp); - const double exp = 0.25; - - std::cout << std::endl - << "Benchmarking base=" << base << ", exp=" << exp << "..." - << std::endl; - std::cout << "- void: " - << Acts::Test::microBenchmark([&] { return 0; }, nReps) - << std::endl; - std::cout << "- std::pow: " - << Acts::Test::microBenchmark([&] { return std::pow(base, exp); }, - nReps) - << std::endl; - std::cout << "- std::exp: " - << Acts::Test::microBenchmark( - [&] { return std::exp(std::log(base) * exp); }, nReps) - << std::endl; - std::cout << "- std::sqrt: " - << Acts::Test::microBenchmark( - [&] { return std::sqrt(std::sqrt(base)); }, nReps) - << std::endl; - std::cout << "- fastPow: " - << Acts::Test::microBenchmark([&] { return fastPow(base, exp); }, - nReps) - << std::endl; - std::cout << "- fastPowMorePrecise: " - << Acts::Test::microBenchmark( - [&] { return fastPowMorePrecise(base, exp); }, nReps) - << std::endl; - std::cout << "- fastPowAnother: " - << Acts::Test::microBenchmark( - [&] { return fastPowAnother(base, exp); }, nReps) - << std::endl; -} - -} // namespace Acts::Test diff --git a/Tests/UnitTests/Core/Utilities/CMakeLists.txt b/Tests/UnitTests/Core/Utilities/CMakeLists.txt index 68b4ba5a655..27b91bfbe29 100644 --- a/Tests/UnitTests/Core/Utilities/CMakeLists.txt +++ b/Tests/UnitTests/Core/Utilities/CMakeLists.txt @@ -54,7 +54,6 @@ target_compile_definitions( add_unittest(ParticleData ParticleDataTests.cpp) add_unittest(Zip ZipTests.cpp) add_unittest(TransformRange TransformRangeTests.cpp) -add_unittest(QuickMath QuickMathTests.cpp) add_unittest(TrackHelpers TrackHelpersTests.cpp) add_unittest(GraphViz GraphVizTests.cpp) diff --git a/Tests/UnitTests/Core/Utilities/QuickMathTests.cpp b/Tests/UnitTests/Core/Utilities/QuickMathTests.cpp deleted file mode 100644 index 6952ea78b14..00000000000 --- a/Tests/UnitTests/Core/Utilities/QuickMathTests.cpp +++ /dev/null @@ -1,64 +0,0 @@ -// 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 -#include - -#include "Acts/Tests/CommonHelpers/FloatComparisons.hpp" -#include "Acts/Utilities/QuickMath.hpp" - -namespace bdata = boost::unit_test::data; - -const auto expDist = bdata::random( - (bdata::engine = std::mt19937{}, bdata::seed = 0, - bdata::distribution = std::uniform_real_distribution(-4, 4))); - -BOOST_AUTO_TEST_SUITE(Utilities) - -BOOST_DATA_TEST_CASE(fastInverseSqrt, expDist ^ bdata::xrange(100), exp, i) { - (void)i; - - const double x = std::pow(10, exp); - - const float fastFloat = Acts::fastInverseSqrt(static_cast(x)); - const double fastDouble = Acts::fastInverseSqrt(x); - - const double stdFloat = 1.0 / std::sqrt(static_cast(x)); - const double stdDouble = 1.0 / std::sqrt(x); - - CHECK_CLOSE_REL(stdFloat, fastFloat, 0.01); - CHECK_CLOSE_REL(stdDouble, fastDouble, 0.01); -} - -BOOST_DATA_TEST_CASE(fastPow, expDist ^ expDist ^ bdata::xrange(100), baseExp, - exp, i) { - (void)i; - - const double base = std::pow(10, baseExp); - - const double fast = Acts::fastPow(base, exp); - const double fastMorePrecise = Acts::fastPowMorePrecise(base, exp); - - const double std = std::pow(base, exp); - - CHECK_CLOSE_REL(fast, std, 0.15); - CHECK_CLOSE_REL(fastMorePrecise, std, 0.1); -} - -// BOOST_AUTO_TEST_CASE(fastPowChart) { -// std::cout << "a ref obs" << std::endl; -// for (double aExp = -4; aExp <= 4; aExp += 0.01) { -// double a = std::pow(10, aExp); -// double ref = std::pow(a, 0.25); -// double obs = Acts::fastPow(a, 0.25); - -// std::cout << a << " " << ref << " " << obs << std::endl; -// } -// } - -BOOST_AUTO_TEST_SUITE_END() From 5ec1437ed89e064b2074778c8827acd6f32e1bed Mon Sep 17 00:00:00 2001 From: Carlo Varni <75478407+CarloVarni@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:06:35 +0200 Subject: [PATCH 12/18] refactor: Do not insert space points if not inside grid boundaries (#3698) Changes: - Calling `grid.isInside` to check if space point falls within the boundaries - Check min and max values are correct (i.e. min < max) - Additional changes to check phi is within `[-std::numbers::pi`, `std::numbers::pi`] - Move config and options values to `double` (also because units are `double`s). Only partial since moving them all requires change in other parts of the code (will follow up in another PR) - Use `std::numbers::pi` instead of `M_PI` In the example, we go back to the previous computation for the `rMiddleSPRange`. Not clear why the `std::floor` was introduced in the first place, going back to that computation while we figure out why --- .../detail/CylindricalSpacePointGrid.hpp | 31 +++++++++++++++++-- .../detail/CylindricalSpacePointGrid.ipp | 14 ++++++--- .../TrackFinding/src/SeedingAlgorithm.cpp | 6 ++-- Examples/Python/python/acts/examples/itk.py | 4 +-- .../python/acts/examples/reconstruction.py | 1 + Examples/Scripts/Python/full_chain_odd.py | 5 +-- 6 files changed, 48 insertions(+), 13 deletions(-) diff --git a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp index 42d60158dd0..5a72449373a 100644 --- a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp +++ b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp @@ -12,6 +12,7 @@ #include "Acts/Seeding/SeedFinderConfig.hpp" #include "Acts/Utilities/Grid.hpp" +#include #include namespace Acts { @@ -56,9 +57,9 @@ struct CylindricalSpacePointGridConfig { // maximum impact parameter in mm float impactMax = 0 * Acts::UnitConstants::mm; // minimum phi value for phiAxis construction - float phiMin = -M_PI; + float phiMin = -std::numbers::pi_v; // maximum phi value for phiAxis construction - float phiMax = M_PI; + float phiMax = std::numbers::pi_v; // Multiplicator for the number of phi-bins. The minimum number of phi-bins // depends on min_pt, magnetic field: 2*M_PI/(minPT particle phi-deflection). // phiBinDeflectionCoverage is a multiplier for this number. If @@ -78,6 +79,7 @@ struct CylindricalSpacePointGridConfig { "Repeated conversion to internal units for " "CylindricalSpacePointGridConfig"); } + using namespace Acts::UnitLiterals; CylindricalSpacePointGridConfig config = *this; config.isInInternalUnits = true; @@ -88,13 +90,36 @@ struct CylindricalSpacePointGridConfig { config.zMin /= 1_mm; config.deltaRMax /= 1_mm; + if (config.phiMin < -std::numbers::pi_v || + config.phiMax > std::numbers::pi_v) { + throw std::runtime_error( + "CylindricalSpacePointGridConfig: phiMin (" + + std::to_string(config.phiMin) + ") and/or phiMax (" + + std::to_string(config.phiMax) + + ") are outside " + "the allowed phi range, defined as " + "[-std::numbers::pi_v, std::numbers::pi_v]"); + } + if (config.phiMin > config.phiMax) { + throw std::runtime_error( + "CylindricalSpacePointGridConfig: phiMin is bigger then phiMax"); + } + if (config.rMin > config.rMax) { + throw std::runtime_error( + "CylindricalSpacePointGridConfig: rMin is bigger then rMax"); + } + if (config.zMin > config.zMax) { + throw std::runtime_error( + "CylindricalSpacePointGridConfig: zMin is bigger than zMax"); + } + return config; } }; struct CylindricalSpacePointGridOptions { // magnetic field - float bFieldInZ = 0; + float bFieldInZ = 0.; bool isInInternalUnits = false; CylindricalSpacePointGridOptions toInternalUnits() const { if (isInInternalUnits) { diff --git a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp index 5d8c886f980..a21c08abd63 100644 --- a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp +++ b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp @@ -167,9 +167,11 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid( // Space points are assumed to be ALREADY CORRECTED for beamspot position // phi, z and r space point selection comes naturally from the - // grid axis definition. No need to explicitly cut on those values. + // grid axis definition. Calling `isInside` will let us know if we are + // inside the grid range. // If a space point is outside the validity range of these quantities - // it goes in an over- or under-flow bin. + // it goes in an over- or under-flow bin. We want to avoid to consider those + // and skip some computations. // Additional cuts can be applied by customizing the space point selector // in the config object. @@ -189,8 +191,12 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid( } // fill rbins into grid - std::size_t globIndex = grid.globalBinFromPosition( - Acts::Vector3{sp.phi(), sp.z(), sp.radius()}); + Acts::Vector3 position(sp.phi(), sp.z(), sp.radius()); + if (!grid.isInside(position)) { + continue; + } + + std::size_t globIndex = grid.globalBinFromPosition(position); auto& rbin = grid.at(globIndex); rbin.push_back(&sp); diff --git a/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp b/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp index 14b89d848ca..d8bd6265f27 100644 --- a/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp +++ b/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp @@ -274,8 +274,10 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithm::execute( /// variable middle SP radial region of interest const Acts::Range1D rMiddleSPRange( - minRange + m_cfg.seedFinderConfig.deltaRMiddleMinSPRange, - maxRange - m_cfg.seedFinderConfig.deltaRMiddleMaxSPRange); + std::floor(minRange / 2) * 2 + + m_cfg.seedFinderConfig.deltaRMiddleMinSPRange, + std::floor(maxRange / 2) * 2 - + m_cfg.seedFinderConfig.deltaRMiddleMaxSPRange); // run the seeding static thread_local std::vector seeds; diff --git a/Examples/Python/python/acts/examples/itk.py b/Examples/Python/python/acts/examples/itk.py index b9d92b04001..b8e2eba8796 100644 --- a/Examples/Python/python/acts/examples/itk.py +++ b/Examples/Python/python/acts/examples/itk.py @@ -372,8 +372,8 @@ def itkSeedingAlgConfig( ) zOriginWeightFactor = 1 compatSeedWeight = 100 - phiMin = 0 - phiMax = 2 * math.pi + phiMin = -math.pi + phiMax = math.pi phiBinDeflectionCoverage = 3 numPhiNeighbors = 1 maxPhiBins = 200 diff --git a/Examples/Python/python/acts/examples/reconstruction.py b/Examples/Python/python/acts/examples/reconstruction.py index ab12005af86..7ac0989744f 100644 --- a/Examples/Python/python/acts/examples/reconstruction.py +++ b/Examples/Python/python/acts/examples/reconstruction.py @@ -2,6 +2,7 @@ from typing import Optional, Union, List from enum import Enum from collections import namedtuple +import math import acts import acts.examples diff --git a/Examples/Scripts/Python/full_chain_odd.py b/Examples/Scripts/Python/full_chain_odd.py index e6da5eee4dd..22f20d3b257 100755 --- a/Examples/Scripts/Python/full_chain_odd.py +++ b/Examples/Scripts/Python/full_chain_odd.py @@ -3,6 +3,7 @@ import os import argparse import pathlib +import math import acts import acts.examples @@ -412,8 +413,8 @@ maxSharedTracksPerMeasurement=2, pTMax=1400, pTMin=0.5, - phiMax=3.14, - phiMin=-3.14, + phiMax=math.pi, + phiMin=-math.pi, etaMax=4, etaMin=-4, useAmbiguityFunction=False, From 4c7ae2b757bf74620db4c84c9af9e661c4964aaf Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 9 Oct 2024 14:21:11 +0200 Subject: [PATCH 13/18] test: Add GeoModel plugin to Downstream project test (#3704) This would have caught the issue fixed in #3699. --- Tests/DownstreamProject/CMakeLists.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Tests/DownstreamProject/CMakeLists.txt b/Tests/DownstreamProject/CMakeLists.txt index d41277017a5..0cb5ca1f9ea 100644 --- a/Tests/DownstreamProject/CMakeLists.txt +++ b/Tests/DownstreamProject/CMakeLists.txt @@ -53,3 +53,10 @@ if(EDM4HEP) find_package(Acts CONFIG REQUIRED COMPONENTS PluginEDM4hep) target_link_libraries(ShowActsVersion PRIVATE ActsPluginEDM4hep) endif() + +option(GEOMODEL "Build with GeoModel" ON) +if(GEOMODEL) + message(STATUS "Adding GeoModel plugin") + find_package(Acts CONFIG REQUIRED COMPONENTS PluginGeoModel) + target_link_libraries(ShowActsVersion PRIVATE ActsPluginGeoModel) +endif() From fc21c841d1a6e63334e152c31ce8c9e3608e3ae5 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 9 Oct 2024 15:28:56 +0200 Subject: [PATCH 14/18] fix: `material_recoding.py` conditional loading of GeoModel (#3703) The previous method of a nested `import` does not actually work. --- Examples/Scripts/Python/material_recording.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Examples/Scripts/Python/material_recording.py b/Examples/Scripts/Python/material_recording.py index ee8910e8ef4..9de5914d45a 100755 --- a/Examples/Scripts/Python/material_recording.py +++ b/Examples/Scripts/Python/material_recording.py @@ -18,6 +18,12 @@ import acts.examples.geant4.dd4hep from acts.examples.odd import getOpenDataDetector +try: + import acts.examples.geant4.geomodel +except ImportError: + # geomodel is optional for this script + pass + u = acts.UnitConstants _material_recording_executed = False @@ -114,8 +120,6 @@ def main(): acts.examples.geant4.GdmlDetectorConstructionFactory(args.input) ) elif args.input.endswith(".sqlite") or args.input.endswith(".db"): - import acts.examples.geant4.geomodel - geoModelTree = acts.geomodel.readFromDb(args.input) detectorConstructionFactory = ( acts.examples.geant4.geomodel.GeoModelDetectorConstructionFactory( From 464fd468fc31c61e030f6db97189bb1c180892ff Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com> Date: Wed, 9 Oct 2024 16:54:58 +0200 Subject: [PATCH 15/18] chore: remove some unnecessary includes (#3700) Builds fine on GCC 14 and Clang 18. --- Core/include/Acts/EventData/SourceLink.hpp | 1 - .../Acts/Seeding/EstimateTrackParamsFromSeed.hpp | 2 -- .../Acts/TrackFinding/CombinatorialKalmanFilter.hpp | 13 ------------- .../TrackFinding/TrackFindingAlgorithm.hpp | 4 ---- .../TrackFinding/src/TrackFindingAlgorithm.cpp | 3 --- .../src/TrackFindingAlgorithmFunction.cpp | 11 +---------- .../src/TrackParamsEstimationAlgorithm.cpp | 3 --- .../DD4hepDetector/src/DD4hepGeometryService.cpp | 1 - Examples/Framework/ML/src/NeuralCalibrator.cpp | 1 + .../EventData/MeasurementCalibration.hpp | 1 - .../include/ActsExamples/EventData/SimSeed.hpp | 1 - .../ActsExamples/Framework/AlgorithmContext.hpp | 2 -- .../include/ActsExamples/Framework/IAlgorithm.hpp | 2 +- 13 files changed, 3 insertions(+), 42 deletions(-) diff --git a/Core/include/Acts/EventData/SourceLink.hpp b/Core/include/Acts/EventData/SourceLink.hpp index 5e032d11e4f..ba1515738e6 100644 --- a/Core/include/Acts/EventData/SourceLink.hpp +++ b/Core/include/Acts/EventData/SourceLink.hpp @@ -14,7 +14,6 @@ #include #include -#include #include #include diff --git a/Core/include/Acts/Seeding/EstimateTrackParamsFromSeed.hpp b/Core/include/Acts/Seeding/EstimateTrackParamsFromSeed.hpp index 4b491c6ddc5..11d38f2eea9 100644 --- a/Core/include/Acts/Seeding/EstimateTrackParamsFromSeed.hpp +++ b/Core/include/Acts/Seeding/EstimateTrackParamsFromSeed.hpp @@ -10,7 +10,6 @@ #include "Acts/Definitions/TrackParametrization.hpp" #include "Acts/Definitions/Units.hpp" -#include "Acts/EventData/Seed.hpp" #include "Acts/Geometry/GeometryContext.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Utilities/Logger.hpp" @@ -20,7 +19,6 @@ #include #include #include -#include namespace Acts { /// @todo: diff --git a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp index b28b7372252..afb74140b8d 100644 --- a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp +++ b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp @@ -11,43 +11,30 @@ // Workaround for building on clang+libstdc++ #include "Acts/Utilities/detail/ReferenceWrapperAnyCompat.hpp" -#include "Acts/Definitions/Algebra.hpp" #include "Acts/Definitions/Common.hpp" -#include "Acts/EventData/MeasurementHelpers.hpp" #include "Acts/EventData/MultiTrajectory.hpp" #include "Acts/EventData/MultiTrajectoryHelpers.hpp" -#include "Acts/EventData/ProxyAccessor.hpp" -#include "Acts/EventData/TrackContainer.hpp" #include "Acts/EventData/TrackParameters.hpp" #include "Acts/EventData/TrackStatePropMask.hpp" #include "Acts/EventData/Types.hpp" #include "Acts/Geometry/GeometryContext.hpp" #include "Acts/MagneticField/MagneticFieldContext.hpp" -#include "Acts/Material/MaterialSlab.hpp" #include "Acts/Propagator/ActorList.hpp" #include "Acts/Propagator/ConstrainedStep.hpp" -#include "Acts/Propagator/Propagator.hpp" #include "Acts/Propagator/StandardAborters.hpp" #include "Acts/Propagator/detail/LoopProtection.hpp" #include "Acts/Propagator/detail/PointwiseMaterialInteraction.hpp" -#include "Acts/Surfaces/BoundaryTolerance.hpp" #include "Acts/TrackFinding/CombinatorialKalmanFilterError.hpp" #include "Acts/TrackFitting/KalmanFitter.hpp" #include "Acts/TrackFitting/detail/VoidFitterComponents.hpp" #include "Acts/Utilities/CalibrationContext.hpp" -#include "Acts/Utilities/HashedString.hpp" #include "Acts/Utilities/Logger.hpp" #include "Acts/Utilities/Result.hpp" -#include "Acts/Utilities/TrackHelpers.hpp" -#include "Acts/Utilities/Zip.hpp" #include #include #include -#include -#include #include -#include namespace Acts { diff --git a/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/TrackFindingAlgorithm.hpp b/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/TrackFindingAlgorithm.hpp index 8a36481fc41..f63581470e6 100644 --- a/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/TrackFindingAlgorithm.hpp +++ b/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/TrackFindingAlgorithm.hpp @@ -8,12 +8,10 @@ #pragma once -#include "Acts/EventData/MultiTrajectory.hpp" #include "Acts/EventData/SourceLink.hpp" #include "Acts/EventData/TrackContainer.hpp" #include "Acts/EventData/TrackProxy.hpp" #include "Acts/EventData/VectorMultiTrajectory.hpp" -#include "Acts/EventData/VectorTrackContainer.hpp" #include "Acts/Geometry/TrackingGeometry.hpp" #include "Acts/TrackFinding/CombinatorialKalmanFilter.hpp" #include "Acts/TrackFinding/MeasurementSelector.hpp" @@ -28,11 +26,9 @@ #include "ActsExamples/Framework/DataHandle.hpp" #include "ActsExamples/Framework/IAlgorithm.hpp" #include "ActsExamples/Framework/ProcessCode.hpp" -#include "ActsExamples/MagneticField/MagneticField.hpp" #include #include -#include #include #include #include diff --git a/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp b/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp index a6d5efc69e6..cca2fe7dfef 100644 --- a/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp +++ b/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp @@ -28,13 +28,10 @@ #include "Acts/Surfaces/Surface.hpp" #include "Acts/TrackFinding/CombinatorialKalmanFilter.hpp" #include "Acts/TrackFitting/GainMatrixUpdater.hpp" -#include "Acts/TrackFitting/KalmanFitter.hpp" -#include "Acts/Utilities/Delegate.hpp" #include "Acts/Utilities/Enumerate.hpp" #include "Acts/Utilities/Logger.hpp" #include "Acts/Utilities/TrackHelpers.hpp" #include "ActsExamples/EventData/IndexSourceLink.hpp" -#include "ActsExamples/EventData/Measurement.hpp" #include "ActsExamples/EventData/MeasurementCalibration.hpp" #include "ActsExamples/EventData/SimSeed.hpp" #include "ActsExamples/EventData/Track.hpp" diff --git a/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithmFunction.cpp b/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithmFunction.cpp index aaed21134d3..65e0cf41970 100644 --- a/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithmFunction.cpp +++ b/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithmFunction.cpp @@ -6,26 +6,17 @@ // 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/Definitions/Direction.hpp" -#include "Acts/EventData/MultiTrajectory.hpp" #include "Acts/EventData/TrackContainer.hpp" -#include "Acts/EventData/TrackStatePropMask.hpp" -#include "Acts/EventData/VectorMultiTrajectory.hpp" -#include "Acts/EventData/VectorTrackContainer.hpp" #include "Acts/Propagator/Navigator.hpp" #include "Acts/Propagator/Propagator.hpp" #include "Acts/Propagator/SympyStepper.hpp" #include "Acts/TrackFinding/CombinatorialKalmanFilter.hpp" -#include "Acts/TrackFitting/GainMatrixSmoother.hpp" #include "Acts/Utilities/Logger.hpp" #include "ActsExamples/EventData/Track.hpp" #include "ActsExamples/TrackFinding/TrackFindingAlgorithm.hpp" -#include -#include #include #include -#include namespace Acts { class MagneticFieldProvider; @@ -53,7 +44,7 @@ struct TrackFinderFunctionImpl ActsExamples::TrackProxy rootBranch) const override { return trackFinder.findTracks(initialParameters, options, tracks, rootBranch); - }; + } }; } // namespace diff --git a/Examples/Algorithms/TrackFinding/src/TrackParamsEstimationAlgorithm.cpp b/Examples/Algorithms/TrackFinding/src/TrackParamsEstimationAlgorithm.cpp index 880e1571517..a54b10d2e7d 100644 --- a/Examples/Algorithms/TrackFinding/src/TrackParamsEstimationAlgorithm.cpp +++ b/Examples/Algorithms/TrackFinding/src/TrackParamsEstimationAlgorithm.cpp @@ -10,12 +10,9 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/Definitions/TrackParametrization.hpp" -#include "Acts/EventData/ParticleHypothesis.hpp" #include "Acts/EventData/Seed.hpp" -#include "Acts/EventData/SourceLink.hpp" #include "Acts/Geometry/GeometryIdentifier.hpp" #include "Acts/Geometry/TrackingGeometry.hpp" -#include "Acts/MagneticField/MagneticFieldProvider.hpp" #include "Acts/Seeding/EstimateTrackParamsFromSeed.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Utilities/Result.hpp" diff --git a/Examples/Detectors/DD4hepDetector/src/DD4hepGeometryService.cpp b/Examples/Detectors/DD4hepDetector/src/DD4hepGeometryService.cpp index 6856ff94a5c..2912e39b757 100644 --- a/Examples/Detectors/DD4hepDetector/src/DD4hepGeometryService.cpp +++ b/Examples/Detectors/DD4hepDetector/src/DD4hepGeometryService.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include diff --git a/Examples/Framework/ML/src/NeuralCalibrator.cpp b/Examples/Framework/ML/src/NeuralCalibrator.cpp index ee1a8566aa4..68d24d35468 100644 --- a/Examples/Framework/ML/src/NeuralCalibrator.cpp +++ b/Examples/Framework/ML/src/NeuralCalibrator.cpp @@ -14,6 +14,7 @@ #include "Acts/Utilities/CalibrationContext.hpp" #include "Acts/Utilities/Helpers.hpp" #include "Acts/Utilities/UnitVectors.hpp" +#include "ActsExamples/EventData/IndexSourceLink.hpp" #include "ActsExamples/EventData/Measurement.hpp" #include diff --git a/Examples/Framework/include/ActsExamples/EventData/MeasurementCalibration.hpp b/Examples/Framework/include/ActsExamples/EventData/MeasurementCalibration.hpp index 248edacc032..f0b61363fba 100644 --- a/Examples/Framework/include/ActsExamples/EventData/MeasurementCalibration.hpp +++ b/Examples/Framework/include/ActsExamples/EventData/MeasurementCalibration.hpp @@ -14,7 +14,6 @@ #include "Acts/Geometry/GeometryContext.hpp" #include "Acts/Utilities/CalibrationContext.hpp" #include "ActsExamples/EventData/Cluster.hpp" -#include "ActsExamples/EventData/IndexSourceLink.hpp" #include #include diff --git a/Examples/Framework/include/ActsExamples/EventData/SimSeed.hpp b/Examples/Framework/include/ActsExamples/EventData/SimSeed.hpp index 627b160b08f..5c211673f2a 100644 --- a/Examples/Framework/include/ActsExamples/EventData/SimSeed.hpp +++ b/Examples/Framework/include/ActsExamples/EventData/SimSeed.hpp @@ -11,7 +11,6 @@ #include "Acts/EventData/Seed.hpp" #include "ActsExamples/EventData/SimSpacePoint.hpp" -#include #include namespace ActsExamples { diff --git a/Examples/Framework/include/ActsExamples/Framework/AlgorithmContext.hpp b/Examples/Framework/include/ActsExamples/Framework/AlgorithmContext.hpp index f78e5b9c798..c70eaea6539 100644 --- a/Examples/Framework/include/ActsExamples/Framework/AlgorithmContext.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/AlgorithmContext.hpp @@ -13,8 +13,6 @@ #include #include -#include - namespace ActsExamples { class WhiteBoard; diff --git a/Examples/Framework/include/ActsExamples/Framework/IAlgorithm.hpp b/Examples/Framework/include/ActsExamples/Framework/IAlgorithm.hpp index 46856ed7103..77fd2b41c98 100644 --- a/Examples/Framework/include/ActsExamples/Framework/IAlgorithm.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/IAlgorithm.hpp @@ -44,7 +44,7 @@ class IAlgorithm : public SequenceElement { /// @param context The algorithm context ProcessCode internalExecute(const AlgorithmContext& context) final { return execute(context); - }; + } /// Initialize the algorithm ProcessCode initialize() override { return ProcessCode::SUCCESS; } From 94698b3af583afbac4c3bb18018c7dde16477945 Mon Sep 17 00:00:00 2001 From: Benjamin Huth <37871400+benjaminhuth@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:39:36 +0200 Subject: [PATCH 16/18] fix: Copy whole trackstate did not copy calibrated (#3693) Know your defaults... --- .../Acts/EventData/TrackStateProxy.hpp | 3 ++- .../Core/EventData/TrackTestsExtra.cpp | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Core/include/Acts/EventData/TrackStateProxy.hpp b/Core/include/Acts/EventData/TrackStateProxy.hpp index bed0061ecbf..34de86cd3d7 100644 --- a/Core/include/Acts/EventData/TrackStateProxy.hpp +++ b/Core/include/Acts/EventData/TrackStateProxy.hpp @@ -1043,8 +1043,9 @@ class TrackStateProxy { jacobian() = other.jacobian(); } + // NOTE: we should not check hasCalibrated on this, since it + // may be not yet allocated if (ACTS_CHECK_BIT(mask, PM::Calibrated) && - has() && other.template has()) { allocateCalibrated(other.calibratedSize()); diff --git a/Tests/UnitTests/Core/EventData/TrackTestsExtra.cpp b/Tests/UnitTests/Core/EventData/TrackTestsExtra.cpp index 2c16463ebe1..ca444466216 100644 --- a/Tests/UnitTests/Core/EventData/TrackTestsExtra.cpp +++ b/Tests/UnitTests/Core/EventData/TrackTestsExtra.cpp @@ -447,4 +447,24 @@ BOOST_AUTO_TEST_CASE(ReverseTrackStates) { } } +BOOST_AUTO_TEST_CASE(CopyTrackProxyCalibrated) { + VectorTrackContainer vtc{}; + VectorMultiTrajectory mtj{}; + TrackContainer tc{vtc, mtj}; + + constexpr static std::size_t kMeasurementSize = 3; + + auto track1 = tc.makeTrack(); + auto ts = track1.appendTrackState(TrackStatePropMask::Calibrated); + ts.allocateCalibrated(kMeasurementSize); + ts.calibrated() = Vector3::Ones(); + ts.calibratedCovariance() = SquareMatrix3::Identity(); + ts.setSubspaceIndices(BoundSubspaceIndices{}); + + auto tsCopy = track1.appendTrackState(TrackStatePropMask::Calibrated); + tsCopy.copyFrom(ts, TrackStatePropMask::Calibrated, false); + + BOOST_CHECK_EQUAL(ts.calibratedSize(), tsCopy.calibratedSize()); +} + BOOST_AUTO_TEST_SUITE_END() From 333d1a203e8947ccbe974e4cab41176c5fc614df Mon Sep 17 00:00:00 2001 From: Carlo Varni <75478407+CarloVarni@users.noreply.github.com> Date: Wed, 9 Oct 2024 20:11:12 +0200 Subject: [PATCH 17/18] fix: 3ul instead of 3 for size_t (#3706) Requirement tests an `std::size_t` so it should be compared to another `std::size_t` --- Core/include/Acts/EventData/Seed.hpp | 2 +- Core/include/Acts/EventData/Seed.ipp | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Core/include/Acts/EventData/Seed.hpp b/Core/include/Acts/EventData/Seed.hpp index bd159eaa54a..db93b319dcb 100644 --- a/Core/include/Acts/EventData/Seed.hpp +++ b/Core/include/Acts/EventData/Seed.hpp @@ -14,7 +14,7 @@ namespace Acts { template - requires(N >= 3) + requires(N >= 3ul) class Seed { public: using value_type = external_spacepoint_t; diff --git a/Core/include/Acts/EventData/Seed.ipp b/Core/include/Acts/EventData/Seed.ipp index 5f66aa06772..74f2c0e4169 100644 --- a/Core/include/Acts/EventData/Seed.ipp +++ b/Core/include/Acts/EventData/Seed.ipp @@ -9,7 +9,7 @@ namespace Acts { template - requires(N >= 3) + requires(N >= 3ul) template requires(sizeof...(args_t) == N) && (std::same_as && ...) @@ -17,32 +17,32 @@ Seed::Seed(const args_t&... points) : m_spacepoints({&points...}) {} template - requires(N >= 3) + requires(N >= 3ul) void Seed::setVertexZ(float vertex) { m_vertexZ = vertex; } template - requires(N >= 3) + requires(N >= 3ul) void Seed::setQuality(float seedQuality) { m_seedQuality = seedQuality; } template - requires(N >= 3) + requires(N >= 3ul) const std::array& Seed::sp() const { return m_spacepoints; } template - requires(N >= 3) + requires(N >= 3ul) float Seed::z() const { return m_vertexZ; } template - requires(N >= 3) + requires(N >= 3ul) float Seed::seedQuality() const { return m_seedQuality; } From a9ed41f29c7cdfb8a2c88190d26cfb0a1b14f742 Mon Sep 17 00:00:00 2001 From: Tim Adye Date: Wed, 9 Oct 2024 20:25:05 +0100 Subject: [PATCH 18/18] fix: fix name BranchState (#3707) Fix spelling `BrachState` → `BranchState`. --- .../TrackFinding/src/TrackFindingAlgorithm.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp b/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp index cca2fe7dfef..ed9918c4f84 100644 --- a/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp +++ b/Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp @@ -174,13 +174,13 @@ class BranchStopper { using BranchStopperResult = Acts::CombinatorialKalmanFilterBranchStopperResult; - struct BrachState { + struct BranchState { std::size_t nPixelHoles = 0; std::size_t nStripHoles = 0; }; - static constexpr Acts::ProxyAccessor branchStateAccessor = - Acts::ProxyAccessor(Acts::hashString("MyBranchState")); + static constexpr Acts::ProxyAccessor branchStateAccessor = + Acts::ProxyAccessor(Acts::hashString("MyBranchState")); mutable std::atomic m_nStoppedBranches{0}; @@ -400,8 +400,8 @@ ProcessCode TrackFindingAlgorithm::execute(const AlgorithmContext& ctx) const { TrackContainer tracksTemp(trackContainerTemp, trackStateContainerTemp); // Note that not all backends support PODs as column types - tracks.addColumn("MyBranchState"); - tracksTemp.addColumn("MyBranchState"); + tracks.addColumn("MyBranchState"); + tracksTemp.addColumn("MyBranchState"); tracks.addColumn("trackGroup"); tracksTemp.addColumn("trackGroup");