From 6dc1e8fdebde0a798ea3a4ca9313515bcc5cb9ce Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 20 Sep 2024 11:44:07 +0200 Subject: [PATCH 01/25] refactor: Initial Gen3 portal support for `NavigationStream` --- .../Acts/Navigation/NavigationStream.hpp | 14 ++-- Core/src/Navigation/NavigationStream.cpp | 67 +++++++++++-------- .../Core/Navigation/NavigationStreamTests.cpp | 9 +-- 3 files changed, 53 insertions(+), 37 deletions(-) diff --git a/Core/include/Acts/Navigation/NavigationStream.hpp b/Core/include/Acts/Navigation/NavigationStream.hpp index 71443e48915..7e9c51dc7f9 100644 --- a/Core/include/Acts/Navigation/NavigationStream.hpp +++ b/Core/include/Acts/Navigation/NavigationStream.hpp @@ -10,9 +10,11 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/Geometry/GeometryContext.hpp" +#include "Acts/Geometry/Portal.hpp" #include "Acts/Surfaces/BoundaryTolerance.hpp" #include "Acts/Utilities/Intersection.hpp" +#include #include #include @@ -22,7 +24,6 @@ namespace Acts { namespace Experimental { class Portal; } -using namespace Experimental; class Surface; @@ -55,6 +56,7 @@ class NavigationStream { ObjectIntersection intersection = ObjectIntersection::invalid(); /// The portal + const Acts::Experimental::Portal* gen2Portal = nullptr; const Portal* portal = nullptr; /// The boundary tolerance BoundaryTolerance bTolerance = BoundaryTolerance::None(); @@ -116,25 +118,27 @@ class NavigationStream { /// /// @param surface the surface to be filled /// @param bTolerance the boundary tolerance used for the intersection - void addSurfaceCandidate(const Surface* surface, + void addSurfaceCandidate(const Surface& surface, const BoundaryTolerance& bTolerance); /// Fill n surfaces into the candidate vector /// /// @param surfaces the surfaces that are filled in /// @param bTolerance the boundary tolerance used for the intersection - void addSurfaceCandidates(const std::vector& surfaces, + void addSurfaceCandidates(std::span surfaces, const BoundaryTolerance& bTolerance); /// Fill one portal into the candidate vector /// + void addPortalCandidate(const Experimental::Portal& portal); /// @param portal the portals that are filled in - void addPortalCandidate(const Portal* portal); + + void addPortalCandidate(const Portal& portal); /// Fill n portals into the candidate vector /// /// @param portals the portals that are filled in - void addPortalCandidates(const std::vector& portals); + void addPortalCandidates(std::span portals); /// Initialize the stream from a query point /// diff --git a/Core/src/Navigation/NavigationStream.cpp b/Core/src/Navigation/NavigationStream.cpp index bf15367cd3b..62ec7eb11eb 100644 --- a/Core/src/Navigation/NavigationStream.cpp +++ b/Core/src/Navigation/NavigationStream.cpp @@ -36,7 +36,7 @@ bool Acts::NavigationStream::initialize(const GeometryContext& gctx, // A container collecting additional candidates from multiple // valid interseciton std::vector additionalCandidates = {}; - for (auto& [sIntersection, portal, bTolerance] : m_candidates) { + for (auto& [sIntersection, gen2Portal, portal, bTolerance] : m_candidates) { // Get the surface from the object intersection const Surface* surface = sIntersection.object(); // Intersect the surface @@ -58,7 +58,10 @@ bool Acts::NavigationStream::initialize(const GeometryContext& gctx, originalCandidateUpdated = true; } else { additionalCandidates.emplace_back( - Candidate{rsIntersection, portal, bTolerance}); + Candidate{.intersection = rsIntersection, + .gen2Portal = gen2Portal, + .portal = portal, + .bTolerance = bTolerance}); } } } @@ -74,7 +77,7 @@ bool Acts::NavigationStream::initialize(const GeometryContext& gctx, // The we find the first invalid candidate auto firstInvalid = std::ranges::find_if(m_candidates, [](const Candidate& a) { - const auto& [aIntersection, aPortal, aTolerance] = a; + const auto& [aIntersection, aGen2Portal, aPortal, aTolerance] = a; return !aIntersection.isValid(); }); @@ -91,29 +94,25 @@ bool Acts::NavigationStream::initialize(const GeometryContext& gctx, bool Acts::NavigationStream::update(const GeometryContext& gctx, const QueryPoint& queryPoint, ActsScalar onSurfaceTolerance) { - // Position and direction from the query point - const Vector3& position = queryPoint.position; - const Vector3& direction = queryPoint.direction; - // Loop over the (currently valid) candidates and update for (; m_currentIndex < m_candidates.size(); ++m_currentIndex) { // Get the candidate, and resolve the tuple Candidate& candidate = currentCandidate(); - auto& [sIntersection, portal, bTolerance] = candidate; // Get the surface from the object intersection - const Surface* surface = sIntersection.object(); + const Surface* surface = candidate.intersection.object(); // (re-)Intersect the surface - auto multiIntersection = surface->intersect(gctx, position, direction, - bTolerance, onSurfaceTolerance); + auto multiIntersection = + surface->intersect(gctx, queryPoint.position, queryPoint.direction, + candidate.bTolerance, onSurfaceTolerance); // Split them into valid intersections for (const auto& rsIntersection : multiIntersection.split()) { // Skip wrong index solution - if (rsIntersection.index() != sIntersection.index()) { + if (rsIntersection.index() != candidate.intersection.index()) { continue; } // Valid solution is either on surface or updates the distance if (rsIntersection.isValid()) { - sIntersection = rsIntersection; + candidate.intersection = rsIntersection; return true; } } @@ -123,31 +122,43 @@ bool Acts::NavigationStream::update(const GeometryContext& gctx, } void Acts::NavigationStream::addSurfaceCandidate( - const Surface* surface, const BoundaryTolerance& bTolerance) { - m_candidates.emplace_back(Candidate{ - ObjectIntersection::invalid(surface), nullptr, bTolerance}); + const Surface& surface, const BoundaryTolerance& bTolerance) { + m_candidates.emplace_back( + Candidate{.intersection = ObjectIntersection::invalid(&surface), + .bTolerance = bTolerance}); } void Acts::NavigationStream::addSurfaceCandidates( - const std::vector& surfaces, - const BoundaryTolerance& bTolerance) { + std::span surfaces, const BoundaryTolerance& bTolerance) { std::ranges::for_each(surfaces, [&](const auto* surface) { - m_candidates.emplace_back(Candidate{ - ObjectIntersection::invalid(surface), nullptr, bTolerance}); + m_candidates.emplace_back( + Candidate{.intersection = ObjectIntersection::invalid(surface), + .bTolerance = bTolerance}); }); } -void Acts::NavigationStream::addPortalCandidate(const Portal* portal) { - m_candidates.emplace_back( - Candidate{ObjectIntersection::invalid(&(portal->surface())), - portal, BoundaryTolerance::None()}); +void Acts::NavigationStream::addPortalCandidate( + const Experimental::Portal& portal) { + m_candidates.emplace_back(Candidate{ + .intersection = ObjectIntersection::invalid(&portal.surface()), + .gen2Portal = &portal, + .bTolerance = BoundaryTolerance::None()}); +} + +void Acts::NavigationStream::addPortalCandidate(const Portal& portal) { + m_candidates.emplace_back(Candidate{ + .intersection = ObjectIntersection::invalid(&portal.surface()), + .portal = &portal, + .bTolerance = BoundaryTolerance::None()}); } void Acts::NavigationStream::addPortalCandidates( - const std::vector& portals) { + std::span portals) { std::ranges::for_each(portals, [&](const auto& portal) { - m_candidates.emplace_back( - Candidate{ObjectIntersection::invalid(&(portal->surface())), - portal, BoundaryTolerance::None()}); + m_candidates.emplace_back(Candidate{ + .intersection = + ObjectIntersection::invalid(&(portal->surface())), + .gen2Portal = portal, + .bTolerance = BoundaryTolerance::None()}); }); } diff --git a/Tests/UnitTests/Core/Navigation/NavigationStreamTests.cpp b/Tests/UnitTests/Core/Navigation/NavigationStreamTests.cpp index 097dde2ed64..9ca578b0aba 100644 --- a/Tests/UnitTests/Core/Navigation/NavigationStreamTests.cpp +++ b/Tests/UnitTests/Core/Navigation/NavigationStreamTests.cpp @@ -94,7 +94,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializePlanes) { NavigationStream nStreamTemplate; for (const auto& surface : surfaces) { - nStreamTemplate.addSurfaceCandidate(surface.get(), + nStreamTemplate.addSurfaceCandidate(*surface, Acts::BoundaryTolerance::None()); } BOOST_CHECK_EQUAL(nStreamTemplate.remainingCandidates(), 4u); @@ -140,7 +140,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializePlanes) { // (5) Test de-duplication nStream = nStreamTemplate; - nStreamTemplate.addSurfaceCandidate(surfaces[0].get(), + nStreamTemplate.addSurfaceCandidate(*surfaces.at(0), Acts::BoundaryTolerance::None()); // One surface is duplicated in the stream BOOST_CHECK_EQUAL(nStreamTemplate.remainingCandidates(), 5u); @@ -159,7 +159,7 @@ BOOST_AUTO_TEST_CASE(NavigationStream_UpdatePlanes) { // reachable and intersections inside bounds NavigationStream nStreamTemplate; for (const auto& surface : surfaces) { - nStreamTemplate.addSurfaceCandidate(surface.get(), + nStreamTemplate.addSurfaceCandidate(*surface, Acts::BoundaryTolerance::None()); } BOOST_CHECK_EQUAL(nStreamTemplate.remainingCandidates(), 4u); @@ -231,7 +231,8 @@ BOOST_AUTO_TEST_CASE(NavigationStream_InitializeCylinders) { // Let us fill the surfaces into the navigation stream NavigationStream nStreamTemplate; for (const auto& surface : surfaces) { - nStreamTemplate.addSurfaceCandidates({surface.get()}, + const Surface* pointer = surface.get(); + nStreamTemplate.addSurfaceCandidates({&pointer, 1}, Acts::BoundaryTolerance::None()); } BOOST_CHECK_EQUAL(nStreamTemplate.remainingCandidates(), 4u); From c437c736985e2d1301f838446f023b1938844594 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 20 Sep 2024 12:03:48 +0200 Subject: [PATCH 02/25] feat(geo): Initial draft for Gen3 navigation delegate --- Core/include/Acts/Geometry/TrackingVolume.hpp | 11 ++++ .../Acts/Navigation/NavigationDelegate.hpp | 58 +++++++++++++++++++ Core/src/Geometry/TrackingVolume.cpp | 15 +++++ Core/src/Navigation/CMakeLists.txt | 2 +- Core/src/Navigation/NavigationDelegate.cpp | 24 ++++++++ 5 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 Core/include/Acts/Navigation/NavigationDelegate.hpp create mode 100644 Core/src/Navigation/NavigationDelegate.cpp diff --git a/Core/include/Acts/Geometry/TrackingVolume.hpp b/Core/include/Acts/Geometry/TrackingVolume.hpp index c980314fa6d..5d26d45a0b1 100644 --- a/Core/include/Acts/Geometry/TrackingVolume.hpp +++ b/Core/include/Acts/Geometry/TrackingVolume.hpp @@ -19,11 +19,13 @@ #include "Acts/Geometry/TrackingVolumeVisitorConcept.hpp" #include "Acts/Geometry/Volume.hpp" #include "Acts/Material/IVolumeMaterial.hpp" +#include "Acts/Navigation/NavigationDelegate.hpp" #include "Acts/Surfaces/BoundaryTolerance.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Surfaces/SurfaceArray.hpp" #include "Acts/Surfaces/SurfaceVisitorConcept.hpp" #include "Acts/Utilities/BinnedArray.hpp" +#include "Acts/Utilities/Delegate.hpp" #include "Acts/Utilities/Logger.hpp" #include "Acts/Utilities/TransformRange.hpp" #include "Acts/Visualization/ViewConfig.hpp" @@ -498,6 +500,11 @@ class TrackingVolume : public Volume { const ViewConfig& portalViewConfig, const ViewConfig& sensitiveViewConfig) const; + void setNavigationDelegate(std::unique_ptr delegate); + + void updateNavigationState( + Experimental::Gen3Geometry::NavigationState& state) const; + private: void connectDenseBoundarySurfaces( MutableTrackingVolumeVector& confinedDenseVolumes); @@ -561,6 +568,10 @@ class TrackingVolume : public Volume { std::vector> m_volumes; std::vector> m_portals; std::vector> m_surfaces; + + std::unique_ptr m_navigationDelegateInstance; + INavigationDelegate::DelegateType m_navigationDelegate{ + DelegateFuncTag<&INavigationDelegate::noopUpdate>{}}; }; } // namespace Acts diff --git a/Core/include/Acts/Navigation/NavigationDelegate.hpp b/Core/include/Acts/Navigation/NavigationDelegate.hpp new file mode 100644 index 00000000000..9be7b7f88ba --- /dev/null +++ b/Core/include/Acts/Navigation/NavigationDelegate.hpp @@ -0,0 +1,58 @@ +// 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/Navigation/NavigationStream.hpp" +#include "Acts/Surfaces/Surface.hpp" +#include "Acts/Utilities/Delegate.hpp" + +namespace Acts { + +namespace Experimental::Gen3Geometry { +// @NOTE: This type is PRELIMINARY! It does not represent the final +// implementation of the updated navigation! +struct NavigationState { + const TrackingVolume* currentVolume = nullptr; + NavigationStream main; +}; + +} // namespace Experimental::Gen3Geometry + +class INavigationDelegate { + public: + // @NOTE: This interface is PRELIMINARY! It is subject to change! + + using DelegateType = + Delegate; + + static void noopUpdate( + Experimental::Gen3Geometry::NavigationState& /*state*/) {} + + virtual ~INavigationDelegate() = default; + + virtual void connect(DelegateType& delegate) const = 0; +}; + +class TryAllPortalNavigationDelegate final : public INavigationDelegate { + public: + void connect(DelegateType& delegate) const override { + delegate.connect<&TryAllPortalNavigationDelegate::updateState>(this); + } + + TryAllPortalNavigationDelegate(const TrackingVolume& volume) + : m_volume(&volume) {} + + private: + // @NOTE: This implementation is PRELIMINARY! It is subject to change! + void updateState(Experimental::Gen3Geometry::NavigationState& state) const; + + const TrackingVolume* m_volume; +}; + +} // namespace Acts diff --git a/Core/src/Geometry/TrackingVolume.cpp b/Core/src/Geometry/TrackingVolume.cpp index e7d069b80c5..90c9f6e18cf 100644 --- a/Core/src/Geometry/TrackingVolume.cpp +++ b/Core/src/Geometry/TrackingVolume.cpp @@ -735,4 +735,19 @@ void TrackingVolume::visualize(IVisualization3D& helper, } } +void TrackingVolume::setNavigationDelegate( + std::unique_ptr delegate) { + if (delegate == nullptr) { + throw std::invalid_argument("Navigation delegate is nullptr"); + } + + m_navigationDelegateInstance = std::move(delegate); + m_navigationDelegateInstance->connect(m_navigationDelegate); +} + +void TrackingVolume::updateNavigationState( + Experimental::Gen3Geometry::NavigationState& state) const { + m_navigationDelegate(state); +} + } // namespace Acts diff --git a/Core/src/Navigation/CMakeLists.txt b/Core/src/Navigation/CMakeLists.txt index 81cbf616d44..afb65c3aa8b 100644 --- a/Core/src/Navigation/CMakeLists.txt +++ b/Core/src/Navigation/CMakeLists.txt @@ -1 +1 @@ -target_sources(ActsCore PRIVATE NavigationStream.cpp) +target_sources(ActsCore PRIVATE NavigationStream.cpp NavigationDelegate.cpp) diff --git a/Core/src/Navigation/NavigationDelegate.cpp b/Core/src/Navigation/NavigationDelegate.cpp new file mode 100644 index 00000000000..4c8f901327d --- /dev/null +++ b/Core/src/Navigation/NavigationDelegate.cpp @@ -0,0 +1,24 @@ +// 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/Navigation/NavigationDelegate.hpp" + +#include "Acts/Geometry/TrackingVolume.hpp" + +namespace Acts { + +void TryAllPortalNavigationDelegate::updateState( + Experimental::Gen3Geometry::NavigationState& state) const { + assert(m_volume != nullptr); + assert(state.currentVolume == m_volume); + + for (const auto& portal : state.currentVolume->portals()) { + state.main.addPortalCandidate(portal); + }; +} +} // namespace Acts From 05b16ec474f335d90423f76fd12e53a9da905e8a Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 26 Sep 2024 13:15:37 +0200 Subject: [PATCH 03/25] refactor(geo): NavigationDelegate -> NavigationPolicy --- Core/include/Acts/Geometry/TrackingVolume.hpp | 10 +++++----- .../{NavigationDelegate.hpp => NavigationPolicy.hpp} | 10 +++++----- Core/src/Geometry/TrackingVolume.cpp | 12 ++++++------ Core/src/Navigation/CMakeLists.txt | 2 +- .../{NavigationDelegate.cpp => NavigationPolicy.cpp} | 4 ++-- 5 files changed, 19 insertions(+), 19 deletions(-) rename Core/include/Acts/Navigation/{NavigationDelegate.hpp => NavigationPolicy.hpp} (83%) rename Core/src/Navigation/{NavigationDelegate.cpp => NavigationPolicy.cpp} (86%) diff --git a/Core/include/Acts/Geometry/TrackingVolume.hpp b/Core/include/Acts/Geometry/TrackingVolume.hpp index 5d26d45a0b1..c904acee170 100644 --- a/Core/include/Acts/Geometry/TrackingVolume.hpp +++ b/Core/include/Acts/Geometry/TrackingVolume.hpp @@ -19,7 +19,7 @@ #include "Acts/Geometry/TrackingVolumeVisitorConcept.hpp" #include "Acts/Geometry/Volume.hpp" #include "Acts/Material/IVolumeMaterial.hpp" -#include "Acts/Navigation/NavigationDelegate.hpp" +#include "Acts/Navigation/NavigationPolicy.hpp" #include "Acts/Surfaces/BoundaryTolerance.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Surfaces/SurfaceArray.hpp" @@ -500,7 +500,7 @@ class TrackingVolume : public Volume { const ViewConfig& portalViewConfig, const ViewConfig& sensitiveViewConfig) const; - void setNavigationDelegate(std::unique_ptr delegate); + void setNavigationPolicy(std::unique_ptr policy); void updateNavigationState( Experimental::Gen3Geometry::NavigationState& state) const; @@ -569,9 +569,9 @@ class TrackingVolume : public Volume { std::vector> m_portals; std::vector> m_surfaces; - std::unique_ptr m_navigationDelegateInstance; - INavigationDelegate::DelegateType m_navigationDelegate{ - DelegateFuncTag<&INavigationDelegate::noopUpdate>{}}; + std::unique_ptr m_navigationPolicy; + INavigationPolicy::DelegateType m_navigationDelegate{ + DelegateFuncTag<&INavigationPolicy::noopUpdate>{}}; }; } // namespace Acts diff --git a/Core/include/Acts/Navigation/NavigationDelegate.hpp b/Core/include/Acts/Navigation/NavigationPolicy.hpp similarity index 83% rename from Core/include/Acts/Navigation/NavigationDelegate.hpp rename to Core/include/Acts/Navigation/NavigationPolicy.hpp index 9be7b7f88ba..b0fdcda94c1 100644 --- a/Core/include/Acts/Navigation/NavigationDelegate.hpp +++ b/Core/include/Acts/Navigation/NavigationPolicy.hpp @@ -24,7 +24,7 @@ struct NavigationState { } // namespace Experimental::Gen3Geometry -class INavigationDelegate { +class INavigationPolicy { public: // @NOTE: This interface is PRELIMINARY! It is subject to change! @@ -34,18 +34,18 @@ class INavigationDelegate { static void noopUpdate( Experimental::Gen3Geometry::NavigationState& /*state*/) {} - virtual ~INavigationDelegate() = default; + virtual ~INavigationPolicy() = default; virtual void connect(DelegateType& delegate) const = 0; }; -class TryAllPortalNavigationDelegate final : public INavigationDelegate { +class TryAllPortalNavigationPolicy final : public INavigationPolicy { public: void connect(DelegateType& delegate) const override { - delegate.connect<&TryAllPortalNavigationDelegate::updateState>(this); + delegate.connect<&TryAllPortalNavigationPolicy::updateState>(this); } - TryAllPortalNavigationDelegate(const TrackingVolume& volume) + TryAllPortalNavigationPolicy(const TrackingVolume& volume) : m_volume(&volume) {} private: diff --git a/Core/src/Geometry/TrackingVolume.cpp b/Core/src/Geometry/TrackingVolume.cpp index 90c9f6e18cf..f0c5fdc3c97 100644 --- a/Core/src/Geometry/TrackingVolume.cpp +++ b/Core/src/Geometry/TrackingVolume.cpp @@ -735,14 +735,14 @@ void TrackingVolume::visualize(IVisualization3D& helper, } } -void TrackingVolume::setNavigationDelegate( - std::unique_ptr delegate) { - if (delegate == nullptr) { - throw std::invalid_argument("Navigation delegate is nullptr"); +void TrackingVolume::setNavigationPolicy( + std::unique_ptr policy) { + if (policy == nullptr) { + throw std::invalid_argument("Navigation policy is nullptr"); } - m_navigationDelegateInstance = std::move(delegate); - m_navigationDelegateInstance->connect(m_navigationDelegate); + m_navigationPolicy = std::move(policy); + m_navigationPolicy->connect(m_navigationDelegate); } void TrackingVolume::updateNavigationState( diff --git a/Core/src/Navigation/CMakeLists.txt b/Core/src/Navigation/CMakeLists.txt index afb65c3aa8b..6e8898ac57f 100644 --- a/Core/src/Navigation/CMakeLists.txt +++ b/Core/src/Navigation/CMakeLists.txt @@ -1 +1 @@ -target_sources(ActsCore PRIVATE NavigationStream.cpp NavigationDelegate.cpp) +target_sources(ActsCore PRIVATE NavigationStream.cpp NavigationPolicy.cpp) diff --git a/Core/src/Navigation/NavigationDelegate.cpp b/Core/src/Navigation/NavigationPolicy.cpp similarity index 86% rename from Core/src/Navigation/NavigationDelegate.cpp rename to Core/src/Navigation/NavigationPolicy.cpp index 4c8f901327d..f0ca2556621 100644 --- a/Core/src/Navigation/NavigationDelegate.cpp +++ b/Core/src/Navigation/NavigationPolicy.cpp @@ -6,13 +6,13 @@ // 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/Navigation/NavigationDelegate.hpp" +#include "Acts/Navigation/NavigationPolicy.hpp" #include "Acts/Geometry/TrackingVolume.hpp" namespace Acts { -void TryAllPortalNavigationDelegate::updateState( +void TryAllPortalNavigationPolicy::updateState( Experimental::Gen3Geometry::NavigationState& state) const { assert(m_volume != nullptr); assert(state.currentVolume == m_volume); From 93303f5b42ea58f8fc55d28aa761ab0f46f92768 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 26 Sep 2024 13:17:28 +0200 Subject: [PATCH 04/25] refactor(geo): NavigationArguments, policy / multipolicy, interface updates --- Core/include/Acts/Geometry/TrackingVolume.hpp | 21 +-- .../Acts/Navigation/NavigationDelegate.hpp | 26 +++ .../Acts/Navigation/NavigationPolicy.hpp | 152 +++++++++++++++--- Core/src/Geometry/TrackingVolume.cpp | 13 +- Core/src/Navigation/NavigationPolicy.cpp | 8 +- .../UnitTests/Core/Navigation/CMakeLists.txt | 1 + .../Core/Navigation/NavigationPolicyTests.cpp | 112 +++++++++++++ 7 files changed, 289 insertions(+), 44 deletions(-) create mode 100644 Core/include/Acts/Navigation/NavigationDelegate.hpp create mode 100644 Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp diff --git a/Core/include/Acts/Geometry/TrackingVolume.hpp b/Core/include/Acts/Geometry/TrackingVolume.hpp index c904acee170..dab4ecef92c 100644 --- a/Core/include/Acts/Geometry/TrackingVolume.hpp +++ b/Core/include/Acts/Geometry/TrackingVolume.hpp @@ -13,19 +13,16 @@ #include "Acts/Geometry/BoundarySurfaceT.hpp" #include "Acts/Geometry/GeometryContext.hpp" #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" -#include "Acts/Navigation/NavigationPolicy.hpp" -#include "Acts/Surfaces/BoundaryTolerance.hpp" +#include "Acts/Navigation/NavigationDelegate.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Surfaces/SurfaceArray.hpp" #include "Acts/Surfaces/SurfaceVisitorConcept.hpp" #include "Acts/Utilities/BinnedArray.hpp" -#include "Acts/Utilities/Delegate.hpp" #include "Acts/Utilities/Logger.hpp" #include "Acts/Utilities/TransformRange.hpp" #include "Acts/Visualization/ViewConfig.hpp" @@ -37,7 +34,7 @@ #include #include -#include +#include namespace Acts { @@ -45,7 +42,6 @@ class GlueVolumesDescriptor; class VolumeBounds; template struct NavigationOptions; -class GeometryIdentifier; class IMaterialDecorator; class ISurfaceMaterial; class IVolumeMaterial; @@ -53,6 +49,7 @@ class Surface; class TrackingVolume; struct GeometryIdentifierHook; class Portal; +class INavigationPolicy; /// Interface types of the Gen1 geometry model /// @note This interface is being replaced, and is subject to removal @@ -119,8 +116,8 @@ class TrackingVolume : public Volume { ~TrackingVolume() override; TrackingVolume(const TrackingVolume&) = delete; TrackingVolume& operator=(const TrackingVolume&) = delete; - TrackingVolume(TrackingVolume&&) = default; - TrackingVolume& operator=(TrackingVolume&&) = default; + TrackingVolume(TrackingVolume&&); + TrackingVolume& operator=(TrackingVolume&&); /// Constructor for a container Volume /// - vacuum filled volume either as a for other tracking volumes @@ -157,7 +154,6 @@ class TrackingVolume : public Volume { /// @param volumeName is a string identifier TrackingVolume(Volume& volume, const std::string& volumeName = "undefined"); - // @TODO: This needs to be refactored to include Gen3 volumes /// Return the associated sub Volume, returns THIS if no subVolume exists /// @param gctx The current geometry context object, e.g. alignment /// @param position is the global position associated with that search @@ -502,8 +498,7 @@ class TrackingVolume : public Volume { void setNavigationPolicy(std::unique_ptr policy); - void updateNavigationState( - Experimental::Gen3Geometry::NavigationState& state) const; + void updateNavigationState(const NavigationArguments& args) const; private: void connectDenseBoundarySurfaces( @@ -570,8 +565,8 @@ class TrackingVolume : public Volume { std::vector> m_surfaces; std::unique_ptr m_navigationPolicy; - INavigationPolicy::DelegateType m_navigationDelegate{ - DelegateFuncTag<&INavigationPolicy::noopUpdate>{}}; + + NavigationDelegate m_navigationDelegate{}; }; } // namespace Acts diff --git a/Core/include/Acts/Navigation/NavigationDelegate.hpp b/Core/include/Acts/Navigation/NavigationDelegate.hpp new file mode 100644 index 00000000000..a8b5cb40879 --- /dev/null +++ b/Core/include/Acts/Navigation/NavigationDelegate.hpp @@ -0,0 +1,26 @@ +// 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/Definitions/Algebra.hpp" +#include "Acts/Utilities/Delegate.hpp" + +namespace Acts { + +class NavigationStream; + +struct NavigationArguments { + NavigationStream& main; + Vector3 position; + Vector3 direction; +}; + +using NavigationDelegate = OwningDelegate; + +} // namespace Acts diff --git a/Core/include/Acts/Navigation/NavigationPolicy.hpp b/Core/include/Acts/Navigation/NavigationPolicy.hpp index b0fdcda94c1..2411423c1f9 100644 --- a/Core/include/Acts/Navigation/NavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/NavigationPolicy.hpp @@ -8,51 +8,153 @@ #pragma once -#include "Acts/Navigation/NavigationStream.hpp" +#include "Acts/Navigation/NavigationDelegate.hpp" #include "Acts/Surfaces/Surface.hpp" -#include "Acts/Utilities/Delegate.hpp" +#include "Acts/Utilities/DelegateChain.hpp" + +#include +#include +#include namespace Acts { -namespace Experimental::Gen3Geometry { -// @NOTE: This type is PRELIMINARY! It does not represent the final -// implementation of the updated navigation! -struct NavigationState { - const TrackingVolume* currentVolume = nullptr; - NavigationStream main; -}; +class TrackingVolume; +class INavigationPolicy; -} // namespace Experimental::Gen3Geometry +template +concept NavigationPolicyConcept = requires { + requires std::is_base_of_v; + // Has a conforming update method + requires requires(T policy, const NavigationArguments& args) { + { policy.updateState(args) }; + }; +}; class INavigationPolicy { public: - // @NOTE: This interface is PRELIMINARY! It is subject to change! - - using DelegateType = - Delegate; - - static void noopUpdate( - Experimental::Gen3Geometry::NavigationState& /*state*/) {} + static void noopUpdate(const NavigationArguments& /*unused*/) {} virtual ~INavigationPolicy() = default; - virtual void connect(DelegateType& delegate) const = 0; + virtual void connect(NavigationDelegate& delegate) const = 0; + + protected: + template + void connectDefault(NavigationDelegate& delegate) const { + // This cannot be a concept because we use it in CRTP below + const auto* self = static_cast(this); + DelegateChainFactory{delegate}.add<&T::updateState>(self).store(delegate); + } }; class TryAllPortalNavigationPolicy final : public INavigationPolicy { public: - void connect(DelegateType& delegate) const override { - delegate.connect<&TryAllPortalNavigationPolicy::updateState>(this); - } - - TryAllPortalNavigationPolicy(const TrackingVolume& volume) + explicit TryAllPortalNavigationPolicy(const TrackingVolume& volume) : m_volume(&volume) {} - private: // @NOTE: This implementation is PRELIMINARY! It is subject to change! - void updateState(Experimental::Gen3Geometry::NavigationState& state) const; + void updateState(const NavigationArguments& args) const; + + void connect(NavigationDelegate& delegate) const override { + connectDefault(delegate); + } + private: const TrackingVolume* m_volume; }; +static_assert(NavigationPolicyConcept); + +class MultiNavigationPolicyBase : public INavigationPolicy {}; + +template +class MultiNavigationPolicy final : public MultiNavigationPolicyBase { + public: + MultiNavigationPolicy(Policies&&... policies) + : m_policies{std::forward(policies)...} {} + + void connect(NavigationDelegate& delegate) const override { + auto factory = add(DelegateChainFactory{delegate}, + std::index_sequence_for{}); + + factory.store(delegate); + } + + const std::tuple& policies() const { return m_policies; } + + private: + template + constexpr auto add( + auto factory, + std::integer_sequence /*unused*/) const { + return add(factory); + } + + template + constexpr auto add(auto factory) const { + using policy_type = std::tuple_element_t; + auto* policy = &std::get(m_policies); + + auto added = factory.template add<&policy_type::updateState>(policy); + + if constexpr (sizeof...(Is) > 0) { + return add(added); + } else { + return added; + } + } + + std::tuple m_policies; +}; + +template +class NavigationPolicyFactory { + template + using policy_type = + std::conditional_t<(sizeof...(Fs) > 0), + decltype(MultiNavigationPolicy(std::invoke( + std::declval(), + std::declval())...)), + MultiNavigationPolicy<>>; + + public: + template + friend class NavigationPolicyFactory; + + NavigationPolicyFactory() = default; + + template + constexpr auto add(Args&&... args) + requires(std::is_constructible_v) + { + auto factory = [&](const TrackingVolume& volume) { + return P{volume, std::forward(args)...}; + }; + + return NavigationPolicyFactory{ + std::tuple_cat(std::move(m_factories), + std::make_tuple(std::move(factory)))}; + } + + auto operator()(const TrackingVolume& volume) const + requires(sizeof...(Factories) > 0) + { + using policy_type = decltype(MultiNavigationPolicy(std::invoke( + std::declval(), std::declval())...)); + + return std::apply( + [&](auto&&... factories) -> std::unique_ptr { + return std::make_unique( + std::invoke(factories, volume)...); + }, + m_factories); + } + + private: + NavigationPolicyFactory(std::tuple&& factories) + : m_factories(std::move(factories)) {} + + std::tuple m_factories; +}; + } // namespace Acts diff --git a/Core/src/Geometry/TrackingVolume.cpp b/Core/src/Geometry/TrackingVolume.cpp index f0c5fdc3c97..85b5232ffc5 100644 --- a/Core/src/Geometry/TrackingVolume.cpp +++ b/Core/src/Geometry/TrackingVolume.cpp @@ -16,6 +16,7 @@ #include "Acts/Material/IMaterialDecorator.hpp" #include "Acts/Material/IVolumeMaterial.hpp" #include "Acts/Material/ProtoVolumeMaterial.hpp" +#include "Acts/Navigation/NavigationPolicy.hpp" #include "Acts/Propagator/Navigator.hpp" #include "Acts/Surfaces/RegularSurface.hpp" #include "Acts/Surfaces/Surface.hpp" @@ -28,6 +29,8 @@ #include #include +#include + namespace Acts { // constructor for arguments @@ -47,6 +50,10 @@ TrackingVolume::TrackingVolume( createBoundarySurfaces(); interlinkLayers(); connectDenseBoundarySurfaces(denseVolumeVector); + + DelegateChainBuilder{m_navigationDelegate} + .add<&INavigationPolicy::noopUpdate>() + .store(m_navigationDelegate); } TrackingVolume::TrackingVolume(Volume& volume, const std::string& volumeName) @@ -61,6 +68,8 @@ TrackingVolume::TrackingVolume(const Transform3& transform, {}, volumeName) {} TrackingVolume::~TrackingVolume() = default; +TrackingVolume::TrackingVolume(TrackingVolume&&) = default; +TrackingVolume& TrackingVolume::operator=(TrackingVolume&&) = default; const TrackingVolume* TrackingVolume::lowestTrackingVolume( const GeometryContext& gctx, const Vector3& position, @@ -746,8 +755,8 @@ void TrackingVolume::setNavigationPolicy( } void TrackingVolume::updateNavigationState( - Experimental::Gen3Geometry::NavigationState& state) const { - m_navigationDelegate(state); + const NavigationArguments& args) const { + m_navigationDelegate(args); } } // namespace Acts diff --git a/Core/src/Navigation/NavigationPolicy.cpp b/Core/src/Navigation/NavigationPolicy.cpp index f0ca2556621..4e6fd939b7c 100644 --- a/Core/src/Navigation/NavigationPolicy.cpp +++ b/Core/src/Navigation/NavigationPolicy.cpp @@ -9,16 +9,16 @@ #include "Acts/Navigation/NavigationPolicy.hpp" #include "Acts/Geometry/TrackingVolume.hpp" +#include "Acts/Navigation/NavigationStream.hpp" namespace Acts { void TryAllPortalNavigationPolicy::updateState( - Experimental::Gen3Geometry::NavigationState& state) const { + const NavigationArguments& args) const { assert(m_volume != nullptr); - assert(state.currentVolume == m_volume); - for (const auto& portal : state.currentVolume->portals()) { - state.main.addPortalCandidate(portal); + for (const auto& portal : m_volume->portals()) { + args.main.addPortalCandidate(portal); }; } } // namespace Acts diff --git a/Tests/UnitTests/Core/Navigation/CMakeLists.txt b/Tests/UnitTests/Core/Navigation/CMakeLists.txt index edca953321d..a18c2d34c9c 100644 --- a/Tests/UnitTests/Core/Navigation/CMakeLists.txt +++ b/Tests/UnitTests/Core/Navigation/CMakeLists.txt @@ -5,3 +5,4 @@ add_unittest(NavigationStream NavigationStreamTests.cpp) add_unittest(NavigationStateUpdaters NavigationStateUpdatersTests.cpp) add_unittest(DetectorNavigator DetectorNavigatorTests.cpp) add_unittest(MultiWireNavigation MultiWireNavigationTests.cpp) +add_unittest(NavigationPolicy NavigationPolicyTests.cpp) diff --git a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp new file mode 100644 index 00000000000..fcd80a23f1e --- /dev/null +++ b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp @@ -0,0 +1,112 @@ +// 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/Definitions/Units.hpp" +#include "Acts/Geometry/CylinderVolumeBounds.hpp" +#include "Acts/Geometry/TrackingVolume.hpp" +#include "Acts/Navigation/NavigationDelegate.hpp" +#include "Acts/Navigation/NavigationPolicy.hpp" +#include "Acts/Navigation/NavigationStream.hpp" + +using namespace Acts; +using namespace Acts::UnitLiterals; + +BOOST_AUTO_TEST_SUITE(NavigationPolicyTests) + +struct APolicy : public INavigationPolicy { + APolicy(const TrackingVolume& /*volume*/) {} + + void updateState(const NavigationArguments& /*unused*/) const { + const_cast(this)->executed = true; + } + + void connect(NavigationDelegate& delegate) const override { + connectDefault(delegate); + } + + bool executed = false; +}; + +struct BPolicy : public INavigationPolicy { + struct Config { + std::unique_ptr value; + }; + + BPolicy(const TrackingVolume& /*volume*/, Config config) + : m_config(std::move(config)) {} + + void connect(NavigationDelegate& delegate) const override { + connectDefault(delegate); + } + + void updateState(const NavigationArguments& /*unused*/) const { + const_cast(this)->executed = true; + const_cast(this)->value = *m_config.value; + } + + bool executed = false; + int value = 0; + + Config m_config; +}; + +BOOST_AUTO_TEST_CASE(DirectTest) { + TrackingVolume volume{ + Transform3::Identity(), + std::make_shared(250_mm, 400_mm, 310_mm), + "PixelLayer3"}; + + MultiNavigationPolicy policy{ + APolicy{volume}, BPolicy{volume, {.value = std::make_unique(4242)}}}; + + NavigationDelegate delegate; + policy.connect(delegate); + + NavigationStream main; + delegate(NavigationArguments{ + .main = main, .position = Vector3::Zero(), .direction = Vector3::Zero()}); + + BOOST_CHECK(std::get(policy.policies()).executed); + BOOST_CHECK(std::get(policy.policies()).executed); + BOOST_CHECK_EQUAL(std::get(policy.policies()).value, 4242); +} + +BOOST_AUTO_TEST_CASE(FactoryTest) { + TrackingVolume volume{ + Transform3::Identity(), + std::make_shared(250_mm, 400_mm, 310_mm), + "PixelLayer3"}; + + BPolicy::Config config{.value = std::make_unique(42)}; + + std::function(const TrackingVolume&)> + factory = + NavigationPolicyFactory{} + .add() // no arguments + .add(std::move(config)); // config struct as argument + + auto policyBase = factory(volume); + + auto& policy = + dynamic_cast&>(*policyBase); + + NavigationDelegate delegate; + policy.connect(delegate); + + NavigationStream main; + delegate(NavigationArguments{ + .main = main, .position = Vector3::Zero(), .direction = Vector3::Zero()}); + + BOOST_CHECK(std::get(policy.policies()).executed); + BOOST_CHECK(std::get(policy.policies()).executed); + BOOST_CHECK_EQUAL(std::get(policy.policies()).value, 42); +} + +BOOST_AUTO_TEST_SUITE_END() From 6457fa6973e13895f168adbe7ace924d3124d939 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Mon, 30 Sep 2024 14:08:00 +0200 Subject: [PATCH 05/25] feat(nav): Navigation policy factory, split up headers --- .../Acts/Geometry/NavigationPolicyFactory.hpp | 124 ++++++++++++++ .../Acts/Navigation/INavigationPolicy.hpp | 47 +++++ .../Acts/Navigation/MultiNavigationPolicy.hpp | 56 ++++++ .../Acts/Navigation/NavigationDelegate.hpp | 3 + .../Acts/Navigation/NavigationPolicy.hpp | 160 ------------------ .../Navigation/TryAllNavigationPolicies.hpp | 47 +++++ Core/src/Geometry/TrackingVolume.cpp | 2 +- Core/src/Navigation/CMakeLists.txt | 5 +- Core/src/Navigation/NavigationPolicy.cpp | 24 --- .../Navigation/TryAllNavigationPolicies.cpp | 51 ++++++ .../Core/Navigation/NavigationPolicyTests.cpp | 61 +++++-- 11 files changed, 382 insertions(+), 198 deletions(-) create mode 100644 Core/include/Acts/Geometry/NavigationPolicyFactory.hpp create mode 100644 Core/include/Acts/Navigation/INavigationPolicy.hpp create mode 100644 Core/include/Acts/Navigation/MultiNavigationPolicy.hpp delete mode 100644 Core/include/Acts/Navigation/NavigationPolicy.hpp create mode 100644 Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp delete mode 100644 Core/src/Navigation/NavigationPolicy.cpp create mode 100644 Core/src/Navigation/TryAllNavigationPolicies.cpp diff --git a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp new file mode 100644 index 00000000000..22d8d6ca2d6 --- /dev/null +++ b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp @@ -0,0 +1,124 @@ +// 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/Navigation/INavigationPolicy.hpp" +#include "Acts/Navigation/MultiNavigationPolicy.hpp" + +#include +namespace Acts { + +class TrackingVolume; +class INavigationPolicy; + +namespace detail { +template +class NavigationPolicyFactoryImpl; +} + +class NavigationPolicyFactory { + public: + virtual ~NavigationPolicyFactory() = default; + + // This needs to be listed here, but the return type cannot be spelled out + // yet. + static auto make(); + + // This will potentially get serialization interface and deserialization + // functionality + + virtual std::unique_ptr build( + const TrackingVolume& volume) const = 0; +}; + +namespace detail { + +template <> +class NavigationPolicyFactoryImpl<> { + public: + template + friend class NavigationPolicyFactoryImpl; + NavigationPolicyFactoryImpl() = default; + + // Arguments need to be copy constructible because the factory must be able to + // execute multiple times. + template + constexpr auto add(Args&&... args) + requires(std::is_constructible_v && + (std::is_copy_constructible_v && ...)) + { + auto factory = [&](const TrackingVolume& volume) { + return P{volume, std::forward(args)...}; + }; + + return NavigationPolicyFactoryImpl{ + std::tuple_cat(std::make_tuple(std::move(factory)))}; + } +}; + +template +class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { + template + using policy_type_helper = decltype(MultiNavigationPolicy(std::invoke( + std::declval(), std::declval())...)); + + using policy_type = policy_type_helper; + + public: + template + constexpr auto add(Args&&... args) + requires(std::is_constructible_v && + (std::is_copy_constructible_v && ...)) + { + auto factory = [&](const TrackingVolume& volume) { + return P{volume, std::forward(args)...}; + }; + + return NavigationPolicyFactoryImpl{ + std::tuple_cat(std::move(m_factories), + std::make_tuple(std::move(factory)))}; + } + + constexpr std::unique_ptr> + asUniquePtr() && { + return std::make_unique>( + std::move(*this)); + } + + std::unique_ptr operator()(const TrackingVolume& volume) const { + return std::apply( + [&](auto&&... factories) -> std::unique_ptr { + return std::make_unique( + std::invoke(factories, volume)...); + }, + m_factories); + } + + std::unique_ptr build( + const TrackingVolume& volume) const override { + return operator()(volume); + } + + private: + template + friend class NavigationPolicyFactoryImpl; + + NavigationPolicyFactoryImpl(std::tuple&& factories) + : m_factories(std::move(factories)) {} + + std::tuple m_factories; +}; + +} // namespace detail + +inline auto NavigationPolicyFactory::make() { + return detail::NavigationPolicyFactoryImpl<>{}; +} + +} // namespace Acts diff --git a/Core/include/Acts/Navigation/INavigationPolicy.hpp b/Core/include/Acts/Navigation/INavigationPolicy.hpp new file mode 100644 index 00000000000..5584183f730 --- /dev/null +++ b/Core/include/Acts/Navigation/INavigationPolicy.hpp @@ -0,0 +1,47 @@ +// 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/Navigation/NavigationDelegate.hpp" +#include "Acts/Utilities/DelegateChainBuilder.hpp" + +#include + +namespace Acts { + +class TrackingVolume; +class INavigationPolicy; + +template +concept NavigationPolicyConcept = requires { + requires std::is_base_of_v; + // Has a conforming update method + requires requires(T policy, const NavigationArguments& args) { + { policy.updateState(args) }; + }; +}; + +class INavigationPolicy { + public: + static void noopUpdate(const NavigationArguments& /*unused*/) {} + + virtual ~INavigationPolicy() = default; + + virtual void connect(NavigationDelegate& delegate) const = 0; + + protected: + template + void connectDefault(NavigationDelegate& delegate) const { + // This cannot be a concept because we use it in CRTP below + const auto* self = static_cast(this); + DelegateChainBuilder{delegate}.add<&T::updateState>(self).store(delegate); + } +}; + +} // namespace Acts diff --git a/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp b/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp new file mode 100644 index 00000000000..10c0b12e6e7 --- /dev/null +++ b/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp @@ -0,0 +1,56 @@ +// This file is part of the ACTS project. +// +// Copyright (C) 2016 CERN for the benefit of the ACTS project +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +#pragma once + +#include "Acts/Navigation/INavigationPolicy.hpp" + +namespace Acts { + +class MultiNavigationPolicyBase : public INavigationPolicy {}; + +template +class MultiNavigationPolicy final : public MultiNavigationPolicyBase { + public: + MultiNavigationPolicy(Policies&&... policies) + : m_policies{std::forward(policies)...} {} + + void connect(NavigationDelegate& delegate) const override { + auto factory = add(DelegateChainBuilder{delegate}, + std::index_sequence_for{}); + + factory.store(delegate); + } + + const std::tuple& policies() const { return m_policies; } + + private: + template + constexpr auto add( + auto factory, + std::integer_sequence /*unused*/) const { + return add(factory); + } + + template + constexpr auto add(auto factory) const { + using policy_type = std::tuple_element_t; + auto* policy = &std::get(m_policies); + + auto added = factory.template add<&policy_type::updateState>(policy); + + if constexpr (sizeof...(Is) > 0) { + return add(added); + } else { + return added; + } + } + + std::tuple m_policies; +}; +} // namespace Acts diff --git a/Core/include/Acts/Navigation/NavigationDelegate.hpp b/Core/include/Acts/Navigation/NavigationDelegate.hpp index a8b5cb40879..4a9b08cb3fd 100644 --- a/Core/include/Acts/Navigation/NavigationDelegate.hpp +++ b/Core/include/Acts/Navigation/NavigationDelegate.hpp @@ -9,6 +9,7 @@ #pragma once #include "Acts/Definitions/Algebra.hpp" +#include "Acts/Surfaces/BoundaryTolerance.hpp" #include "Acts/Utilities/Delegate.hpp" namespace Acts { @@ -19,6 +20,8 @@ struct NavigationArguments { NavigationStream& main; Vector3 position; Vector3 direction; + + BoundaryTolerance tolerance = BoundaryTolerance::None(); }; using NavigationDelegate = OwningDelegate; diff --git a/Core/include/Acts/Navigation/NavigationPolicy.hpp b/Core/include/Acts/Navigation/NavigationPolicy.hpp deleted file mode 100644 index 2411423c1f9..00000000000 --- a/Core/include/Acts/Navigation/NavigationPolicy.hpp +++ /dev/null @@ -1,160 +0,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/Navigation/NavigationDelegate.hpp" -#include "Acts/Surfaces/Surface.hpp" -#include "Acts/Utilities/DelegateChain.hpp" - -#include -#include -#include - -namespace Acts { - -class TrackingVolume; -class INavigationPolicy; - -template -concept NavigationPolicyConcept = requires { - requires std::is_base_of_v; - // Has a conforming update method - requires requires(T policy, const NavigationArguments& args) { - { policy.updateState(args) }; - }; -}; - -class INavigationPolicy { - public: - static void noopUpdate(const NavigationArguments& /*unused*/) {} - - virtual ~INavigationPolicy() = default; - - virtual void connect(NavigationDelegate& delegate) const = 0; - - protected: - template - void connectDefault(NavigationDelegate& delegate) const { - // This cannot be a concept because we use it in CRTP below - const auto* self = static_cast(this); - DelegateChainFactory{delegate}.add<&T::updateState>(self).store(delegate); - } -}; - -class TryAllPortalNavigationPolicy final : public INavigationPolicy { - public: - explicit TryAllPortalNavigationPolicy(const TrackingVolume& volume) - : m_volume(&volume) {} - - // @NOTE: This implementation is PRELIMINARY! It is subject to change! - void updateState(const NavigationArguments& args) const; - - void connect(NavigationDelegate& delegate) const override { - connectDefault(delegate); - } - - private: - const TrackingVolume* m_volume; -}; - -static_assert(NavigationPolicyConcept); - -class MultiNavigationPolicyBase : public INavigationPolicy {}; - -template -class MultiNavigationPolicy final : public MultiNavigationPolicyBase { - public: - MultiNavigationPolicy(Policies&&... policies) - : m_policies{std::forward(policies)...} {} - - void connect(NavigationDelegate& delegate) const override { - auto factory = add(DelegateChainFactory{delegate}, - std::index_sequence_for{}); - - factory.store(delegate); - } - - const std::tuple& policies() const { return m_policies; } - - private: - template - constexpr auto add( - auto factory, - std::integer_sequence /*unused*/) const { - return add(factory); - } - - template - constexpr auto add(auto factory) const { - using policy_type = std::tuple_element_t; - auto* policy = &std::get(m_policies); - - auto added = factory.template add<&policy_type::updateState>(policy); - - if constexpr (sizeof...(Is) > 0) { - return add(added); - } else { - return added; - } - } - - std::tuple m_policies; -}; - -template -class NavigationPolicyFactory { - template - using policy_type = - std::conditional_t<(sizeof...(Fs) > 0), - decltype(MultiNavigationPolicy(std::invoke( - std::declval(), - std::declval())...)), - MultiNavigationPolicy<>>; - - public: - template - friend class NavigationPolicyFactory; - - NavigationPolicyFactory() = default; - - template - constexpr auto add(Args&&... args) - requires(std::is_constructible_v) - { - auto factory = [&](const TrackingVolume& volume) { - return P{volume, std::forward(args)...}; - }; - - return NavigationPolicyFactory{ - std::tuple_cat(std::move(m_factories), - std::make_tuple(std::move(factory)))}; - } - - auto operator()(const TrackingVolume& volume) const - requires(sizeof...(Factories) > 0) - { - using policy_type = decltype(MultiNavigationPolicy(std::invoke( - std::declval(), std::declval())...)); - - return std::apply( - [&](auto&&... factories) -> std::unique_ptr { - return std::make_unique( - std::invoke(factories, volume)...); - }, - m_factories); - } - - private: - NavigationPolicyFactory(std::tuple&& factories) - : m_factories(std::move(factories)) {} - - std::tuple m_factories; -}; - -} // namespace Acts diff --git a/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp b/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp new file mode 100644 index 00000000000..a2c0ce885c9 --- /dev/null +++ b/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp @@ -0,0 +1,47 @@ +// 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/Navigation/INavigationPolicy.hpp" + +namespace Acts { + +class TrackingVolume; + +// @NOTE: This implementation is PRELIMINARY! It is subject to change! + +class TryAllPortalNavigationPolicy final : public INavigationPolicy { + public: + explicit TryAllPortalNavigationPolicy(const TrackingVolume& volume); + + void updateState(const NavigationArguments& args) const; + + void connect(NavigationDelegate& delegate) const override; + + private: + const TrackingVolume* m_volume; +}; + +static_assert(NavigationPolicyConcept); + +class TryAllSurfaceNavigationPolicy final : public INavigationPolicy { + public: + explicit TryAllSurfaceNavigationPolicy(const TrackingVolume& volume); + + void updateState(const NavigationArguments& args) const; + + void connect(NavigationDelegate& delegate) const override; + + private: + const TrackingVolume* m_volume; +}; + +static_assert(NavigationPolicyConcept); + +} // namespace Acts diff --git a/Core/src/Geometry/TrackingVolume.cpp b/Core/src/Geometry/TrackingVolume.cpp index 85b5232ffc5..8fc7366abdb 100644 --- a/Core/src/Geometry/TrackingVolume.cpp +++ b/Core/src/Geometry/TrackingVolume.cpp @@ -16,7 +16,7 @@ #include "Acts/Material/IMaterialDecorator.hpp" #include "Acts/Material/IVolumeMaterial.hpp" #include "Acts/Material/ProtoVolumeMaterial.hpp" -#include "Acts/Navigation/NavigationPolicy.hpp" +#include "Acts/Navigation/INavigationPolicy.hpp" #include "Acts/Propagator/Navigator.hpp" #include "Acts/Surfaces/RegularSurface.hpp" #include "Acts/Surfaces/Surface.hpp" diff --git a/Core/src/Navigation/CMakeLists.txt b/Core/src/Navigation/CMakeLists.txt index 6e8898ac57f..678db0e4ef7 100644 --- a/Core/src/Navigation/CMakeLists.txt +++ b/Core/src/Navigation/CMakeLists.txt @@ -1 +1,4 @@ -target_sources(ActsCore PRIVATE NavigationStream.cpp NavigationPolicy.cpp) +target_sources( + ActsCore + PRIVATE NavigationStream.cpp TryAllNavigationPolicies.cpp +) diff --git a/Core/src/Navigation/NavigationPolicy.cpp b/Core/src/Navigation/NavigationPolicy.cpp deleted file mode 100644 index 4e6fd939b7c..00000000000 --- a/Core/src/Navigation/NavigationPolicy.cpp +++ /dev/null @@ -1,24 +0,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/Navigation/NavigationPolicy.hpp" - -#include "Acts/Geometry/TrackingVolume.hpp" -#include "Acts/Navigation/NavigationStream.hpp" - -namespace Acts { - -void TryAllPortalNavigationPolicy::updateState( - const NavigationArguments& args) const { - assert(m_volume != nullptr); - - for (const auto& portal : m_volume->portals()) { - args.main.addPortalCandidate(portal); - }; -} -} // namespace Acts diff --git a/Core/src/Navigation/TryAllNavigationPolicies.cpp b/Core/src/Navigation/TryAllNavigationPolicies.cpp new file mode 100644 index 00000000000..b0d9c1251ac --- /dev/null +++ b/Core/src/Navigation/TryAllNavigationPolicies.cpp @@ -0,0 +1,51 @@ +// 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/Navigation/TryAllNavigationPolicies.hpp" + +#include "Acts/Geometry/TrackingVolume.hpp" +#include "Acts/Navigation/NavigationStream.hpp" + +namespace Acts { + +TryAllPortalNavigationPolicy::TryAllPortalNavigationPolicy( + const TrackingVolume& volume) + : m_volume(&volume) {} + +void TryAllPortalNavigationPolicy::updateState( + const NavigationArguments& args) const { + assert(m_volume != nullptr); + + for (const auto& portal : m_volume->portals()) { + args.main.addPortalCandidate(portal); + }; +} + +void TryAllPortalNavigationPolicy::connect(NavigationDelegate& delegate) const { + connectDefault(delegate); +} + +TryAllSurfaceNavigationPolicy::TryAllSurfaceNavigationPolicy( + const TrackingVolume& volume) + : m_volume(&volume) {} + +void TryAllSurfaceNavigationPolicy::updateState( + const NavigationArguments& args) const { + assert(m_volume != nullptr); + + for (const auto& surface : m_volume->surfaces()) { + args.main.addSurfaceCandidate(surface, args.tolerance); + }; +} + +void TryAllSurfaceNavigationPolicy::connect( + NavigationDelegate& delegate) const { + connectDefault(delegate); +} + +} // namespace Acts diff --git a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp index fcd80a23f1e..8f727f6fff5 100644 --- a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp +++ b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp @@ -10,10 +10,13 @@ #include "Acts/Definitions/Units.hpp" #include "Acts/Geometry/CylinderVolumeBounds.hpp" +#include "Acts/Geometry/NavigationPolicyFactory.hpp" #include "Acts/Geometry/TrackingVolume.hpp" +#include "Acts/Navigation/INavigationPolicy.hpp" +#include "Acts/Navigation/MultiNavigationPolicy.hpp" #include "Acts/Navigation/NavigationDelegate.hpp" -#include "Acts/Navigation/NavigationPolicy.hpp" #include "Acts/Navigation/NavigationStream.hpp" +#include "Acts/Navigation/TryAllNavigationPolicies.hpp" using namespace Acts; using namespace Acts::UnitLiterals; @@ -36,11 +39,10 @@ struct APolicy : public INavigationPolicy { struct BPolicy : public INavigationPolicy { struct Config { - std::unique_ptr value; + int value; }; - BPolicy(const TrackingVolume& /*volume*/, Config config) - : m_config(std::move(config)) {} + BPolicy(const TrackingVolume& /*volume*/, Config config) : m_config(config) {} void connect(NavigationDelegate& delegate) const override { connectDefault(delegate); @@ -48,7 +50,7 @@ struct BPolicy : public INavigationPolicy { void updateState(const NavigationArguments& /*unused*/) const { const_cast(this)->executed = true; - const_cast(this)->value = *m_config.value; + const_cast(this)->value = m_config.value; } bool executed = false; @@ -63,8 +65,8 @@ BOOST_AUTO_TEST_CASE(DirectTest) { std::make_shared(250_mm, 400_mm, 310_mm), "PixelLayer3"}; - MultiNavigationPolicy policy{ - APolicy{volume}, BPolicy{volume, {.value = std::make_unique(4242)}}}; + MultiNavigationPolicy policy{APolicy{volume}, + BPolicy{volume, {.value = 4242}}}; NavigationDelegate delegate; policy.connect(delegate); @@ -84,15 +86,15 @@ BOOST_AUTO_TEST_CASE(FactoryTest) { std::make_shared(250_mm, 400_mm, 310_mm), "PixelLayer3"}; - BPolicy::Config config{.value = std::make_unique(42)}; + BPolicy::Config config{.value = 42}; std::function(const TrackingVolume&)> - factory = - NavigationPolicyFactory{} - .add() // no arguments - .add(std::move(config)); // config struct as argument + factory = NavigationPolicyFactory::make() + .add() // no arguments + .add(config); // config struct as argument auto policyBase = factory(volume); + auto policyBase2 = factory(volume); auto& policy = dynamic_cast&>(*policyBase); @@ -107,6 +109,41 @@ BOOST_AUTO_TEST_CASE(FactoryTest) { BOOST_CHECK(std::get(policy.policies()).executed); BOOST_CHECK(std::get(policy.policies()).executed); BOOST_CHECK_EQUAL(std::get(policy.policies()).value, 42); + + auto& policy2 = + dynamic_cast&>(*policyBase2); + + NavigationDelegate delegate2; + policyBase2->connect(delegate2); + + delegate2(NavigationArguments{ + .main = main, .position = Vector3::Zero(), .direction = Vector3::Zero()}); + + BOOST_CHECK(std::get(policy2.policies()).executed); + BOOST_CHECK(std::get(policy2.policies()).executed); + BOOST_CHECK_EQUAL(std::get(policy2.policies()).value, 42); +} + +BOOST_AUTO_TEST_CASE(AsUniquePtrTest) { + TrackingVolume volume{ + Transform3::Identity(), + std::make_shared(250_mm, 400_mm, 310_mm), + "PixelLayer3"}; + + std::unique_ptr factory = + NavigationPolicyFactory::make().add().asUniquePtr(); + + auto policyBase = factory->build(volume); + auto& policy = dynamic_cast&>(*policyBase); + + NavigationDelegate delegate; + policyBase->connect(delegate); + + NavigationStream main; + delegate(NavigationArguments{ + .main = main, .position = Vector3::Zero(), .direction = Vector3::Zero()}); + + BOOST_CHECK(std::get(policy.policies()).executed); } BOOST_AUTO_TEST_SUITE_END() From 67095644ee1105da1386a9515c65add547d98568 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 11 Oct 2024 10:49:15 +0200 Subject: [PATCH 06/25] refactor: NavPolicy factory works with isolated factory --- .../Acts/Geometry/NavigationPolicyFactory.hpp | 38 ++++++++++- .../Core/Navigation/NavigationPolicyTests.cpp | 68 +++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp index 22d8d6ca2d6..639a0cd4d16 100644 --- a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp +++ b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp @@ -11,6 +11,7 @@ #include "Acts/Navigation/INavigationPolicy.hpp" #include "Acts/Navigation/MultiNavigationPolicy.hpp" +#include #include namespace Acts { @@ -39,6 +40,18 @@ class NavigationPolicyFactory { namespace detail { +template +concept NavigationPolicyIsolatedFactoryConcept = requires(F f) { + { + f(std::declval(), std::declval()...) + } -> std::derived_from; + + requires NavigationPolicyConcept(), std::declval()...))>; + + requires(std::is_copy_constructible_v && ...); +}; + template <> class NavigationPolicyFactoryImpl<> { public: @@ -58,7 +71,18 @@ class NavigationPolicyFactoryImpl<> { }; return NavigationPolicyFactoryImpl{ - std::tuple_cat(std::make_tuple(std::move(factory)))}; + std::make_tuple(std::move(factory))}; + } + + template + requires(NavigationPolicyIsolatedFactoryConcept) + constexpr auto add(Fn&& fn, Args&&... args) { + auto factory = [&](const TrackingVolume& volume) { + return fn(volume, std::forward(args)...); + }; + + return NavigationPolicyFactoryImpl{ + std::make_tuple(std::move(factory))}; } }; @@ -85,6 +109,18 @@ class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { std::make_tuple(std::move(factory)))}; } + template + requires(NavigationPolicyIsolatedFactoryConcept) + constexpr auto add(Fn&& fn, Args&&... args) { + auto factory = [&](const TrackingVolume& volume) { + return fn(volume, std::forward(args)...); + }; + + return NavigationPolicyFactoryImpl{ + std::tuple_cat(std::move(m_factories), + std::make_tuple(std::move(factory)))}; + } + constexpr std::unique_ptr> asUniquePtr() && { return std::make_unique>( diff --git a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp index 8f727f6fff5..a35ed3ce147 100644 --- a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp +++ b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp @@ -146,4 +146,72 @@ BOOST_AUTO_TEST_CASE(AsUniquePtrTest) { BOOST_CHECK(std::get(policy.policies()).executed); } +struct CPolicy : public INavigationPolicy {}; + +template +struct CPolicySpecialized : public CPolicy { + struct Config { + T value; + }; + + CPolicySpecialized(const TrackingVolume& /*volume*/, Config config) + : m_config(config) {} + + void connect(NavigationDelegate& delegate) const override { + connectDefault(delegate); + } + + void updateState(const NavigationArguments& /*unused*/) const { + auto* self = const_cast*>(this); + self->executed = true; + self->value = m_config.value; + } + + bool executed = false; + int value = 0; + + Config m_config; +}; + +struct IsolatedConfig { + int value; +}; + +auto makeCPolicy(const TrackingVolume& volume, IsolatedConfig config) { + // I can do arbitrary stuff here + CPolicySpecialized::Config config2{.value = config.value}; + return CPolicySpecialized(volume, config2); +} + +BOOST_AUTO_TEST_CASE(IsolatedFactory) { + TrackingVolume volume{ + Transform3::Identity(), + std::make_shared(250_mm, 400_mm, 310_mm), + "PixelLayer3"}; + + IsolatedConfig config{.value = 44}; + auto factory = + NavigationPolicyFactory::make().add().add(makeCPolicy, config); + + auto factory2 = + NavigationPolicyFactory::make().add(makeCPolicy, config).add(); + + auto policyBase = factory(volume); + auto& policy = + dynamic_cast>&>( + *policyBase); + + NavigationDelegate delegate; + policyBase->connect(delegate); + + NavigationStream main; + delegate(NavigationArguments{ + .main = main, .position = Vector3::Zero(), .direction = Vector3::Zero()}); + + BOOST_CHECK(std::get(policy.policies()).executed); + BOOST_CHECK(std::get>(policy.policies()).executed); + BOOST_CHECK_EQUAL(std::get>(policy.policies()).value, + 44); +} + BOOST_AUTO_TEST_SUITE_END() From f113ce82d83a4f1d6894021f316b5ef5e4d02b46 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Sat, 12 Oct 2024 10:22:11 +0200 Subject: [PATCH 07/25] refactor(nav): NavigationPolicies get geo context + logger in constructor --- .../Acts/Geometry/NavigationPolicyFactory.hpp | 58 +++++++++++++------ .../Navigation/TryAllNavigationPolicies.hpp | 10 +++- .../Navigation/TryAllNavigationPolicies.cpp | 6 +- .../Core/Navigation/NavigationPolicyTests.cpp | 28 +++++---- 4 files changed, 69 insertions(+), 33 deletions(-) diff --git a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp index 639a0cd4d16..3056cb10c77 100644 --- a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp +++ b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp @@ -16,6 +16,8 @@ namespace Acts { class TrackingVolume; +class GeometryContext; +class Logger; class INavigationPolicy; namespace detail { @@ -35,7 +37,8 @@ class NavigationPolicyFactory { // functionality virtual std::unique_ptr build( - const TrackingVolume& volume) const = 0; + const GeometryContext& gctx, const TrackingVolume& volume, + const Logger& logger) const = 0; }; namespace detail { @@ -43,11 +46,15 @@ namespace detail { template concept NavigationPolicyIsolatedFactoryConcept = requires(F f) { { - f(std::declval(), std::declval()...) + f(std::declval(), + std::declval(), std::declval(), + std::declval()...) } -> std::derived_from; requires NavigationPolicyConcept(), std::declval()...))>; + std::declval(), + std::declval(), std::declval(), + std::declval()...))>; requires(std::is_copy_constructible_v && ...); }; @@ -63,11 +70,14 @@ class NavigationPolicyFactoryImpl<> { // execute multiple times. template constexpr auto add(Args&&... args) - requires(std::is_constructible_v && + requires(std::is_constructible_v && (std::is_copy_constructible_v && ...)) { - auto factory = [&](const TrackingVolume& volume) { - return P{volume, std::forward(args)...}; + auto factory = [&](const GeometryContext& gctx, + const TrackingVolume& volume, const Logger& logger) { + return P{gctx, volume, logger, std::forward(args)...}; }; return NavigationPolicyFactoryImpl{ @@ -77,8 +87,9 @@ class NavigationPolicyFactoryImpl<> { template requires(NavigationPolicyIsolatedFactoryConcept) constexpr auto add(Fn&& fn, Args&&... args) { - auto factory = [&](const TrackingVolume& volume) { - return fn(volume, std::forward(args)...); + auto factory = [&](const GeometryContext& gctx, + const TrackingVolume& volume, const Logger& logger) { + return fn(gctx, volume, logger, std::forward(args)...); }; return NavigationPolicyFactoryImpl{ @@ -89,19 +100,24 @@ class NavigationPolicyFactoryImpl<> { template class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { template - using policy_type_helper = decltype(MultiNavigationPolicy(std::invoke( - std::declval(), std::declval())...)); + using policy_type_helper = decltype(MultiNavigationPolicy( + std::invoke(std::declval(), std::declval(), + std::declval(), + std::declval())...)); using policy_type = policy_type_helper; public: template constexpr auto add(Args&&... args) - requires(std::is_constructible_v && + requires(std::is_constructible_v && (std::is_copy_constructible_v && ...)) { - auto factory = [&](const TrackingVolume& volume) { - return P{volume, std::forward(args)...}; + auto factory = [&](const GeometryContext& gctx, + const TrackingVolume& volume, const Logger& logger) { + return P{gctx, volume, logger, std::forward(args)...}; }; return NavigationPolicyFactoryImpl{ @@ -112,8 +128,9 @@ class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { template requires(NavigationPolicyIsolatedFactoryConcept) constexpr auto add(Fn&& fn, Args&&... args) { - auto factory = [&](const TrackingVolume& volume) { - return fn(volume, std::forward(args)...); + auto factory = [&](const GeometryContext& gctx, + const TrackingVolume& volume, const Logger& logger) { + return fn(gctx, volume, logger, std::forward(args)...); }; return NavigationPolicyFactoryImpl{ @@ -127,18 +144,21 @@ class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { std::move(*this)); } - std::unique_ptr operator()(const TrackingVolume& volume) const { + std::unique_ptr operator()(const GeometryContext& gctx, + const TrackingVolume& volume, + const Logger& logger) const { return std::apply( [&](auto&&... factories) -> std::unique_ptr { return std::make_unique( - std::invoke(factories, volume)...); + std::invoke(factories, gctx, volume, logger)...); }, m_factories); } std::unique_ptr build( - const TrackingVolume& volume) const override { - return operator()(volume); + const GeometryContext& gctx, const TrackingVolume& volume, + const Logger& logger) const override { + return operator()(gctx, volume, logger); } private: diff --git a/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp b/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp index a2c0ce885c9..9e15ad35ceb 100644 --- a/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp +++ b/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp @@ -13,12 +13,16 @@ namespace Acts { class TrackingVolume; +class GeometryContext; +class Logger; // @NOTE: This implementation is PRELIMINARY! It is subject to change! class TryAllPortalNavigationPolicy final : public INavigationPolicy { public: - explicit TryAllPortalNavigationPolicy(const TrackingVolume& volume); + explicit TryAllPortalNavigationPolicy(const GeometryContext& gctx, + const TrackingVolume& volume, + const Logger& logger); void updateState(const NavigationArguments& args) const; @@ -32,7 +36,9 @@ static_assert(NavigationPolicyConcept); class TryAllSurfaceNavigationPolicy final : public INavigationPolicy { public: - explicit TryAllSurfaceNavigationPolicy(const TrackingVolume& volume); + explicit TryAllSurfaceNavigationPolicy(const GeometryContext& gctx, + const TrackingVolume& volume, + const Logger& logger); void updateState(const NavigationArguments& args) const; diff --git a/Core/src/Navigation/TryAllNavigationPolicies.cpp b/Core/src/Navigation/TryAllNavigationPolicies.cpp index b0d9c1251ac..1fd77bcc368 100644 --- a/Core/src/Navigation/TryAllNavigationPolicies.cpp +++ b/Core/src/Navigation/TryAllNavigationPolicies.cpp @@ -14,7 +14,8 @@ namespace Acts { TryAllPortalNavigationPolicy::TryAllPortalNavigationPolicy( - const TrackingVolume& volume) + const GeometryContext& /*gctx*/, const TrackingVolume& volume, + const Logger& /*logger*/) : m_volume(&volume) {} void TryAllPortalNavigationPolicy::updateState( @@ -31,7 +32,8 @@ void TryAllPortalNavigationPolicy::connect(NavigationDelegate& delegate) const { } TryAllSurfaceNavigationPolicy::TryAllSurfaceNavigationPolicy( - const TrackingVolume& volume) + const GeometryContext& /*gctx*/, const TrackingVolume& volume, + const Logger& /*logger*/) : m_volume(&volume) {} void TryAllSurfaceNavigationPolicy::updateState( diff --git a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp index a35ed3ce147..ab58afa2a88 100644 --- a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp +++ b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp @@ -23,8 +23,12 @@ using namespace Acts::UnitLiterals; BOOST_AUTO_TEST_SUITE(NavigationPolicyTests) +GeometryContext gctx; +auto logger = getDefaultLogger("NavigationPolicyTests", Logging::VERBOSE); + struct APolicy : public INavigationPolicy { - APolicy(const TrackingVolume& /*volume*/) {} + APolicy(const GeometryContext& /*gctx*/, const TrackingVolume& /*volume*/, + const Logger& /*logger*/) {} void updateState(const NavigationArguments& /*unused*/) const { const_cast(this)->executed = true; @@ -42,7 +46,9 @@ struct BPolicy : public INavigationPolicy { int value; }; - BPolicy(const TrackingVolume& /*volume*/, Config config) : m_config(config) {} + BPolicy(const GeometryContext& /*gctx*/, const TrackingVolume& /*volume*/, + const Logger& /*logger*/, Config config) + : m_config(config) {} void connect(NavigationDelegate& delegate) const override { connectDefault(delegate); @@ -65,8 +71,8 @@ BOOST_AUTO_TEST_CASE(DirectTest) { std::make_shared(250_mm, 400_mm, 310_mm), "PixelLayer3"}; - MultiNavigationPolicy policy{APolicy{volume}, - BPolicy{volume, {.value = 4242}}}; + MultiNavigationPolicy policy{APolicy{gctx, volume, *logger}, + BPolicy{gctx, volume, *logger, {.value = 4242}}}; NavigationDelegate delegate; policy.connect(delegate); @@ -88,13 +94,14 @@ BOOST_AUTO_TEST_CASE(FactoryTest) { BPolicy::Config config{.value = 42}; - std::function(const TrackingVolume&)> + std::function( + const GeometryContext&, const TrackingVolume&, const Logger&)> factory = NavigationPolicyFactory::make() .add() // no arguments .add(config); // config struct as argument - auto policyBase = factory(volume); - auto policyBase2 = factory(volume); + auto policyBase = factory(gctx, volume, *logger); + auto policyBase2 = factory(gctx, volume, *logger); auto& policy = dynamic_cast&>(*policyBase); @@ -133,7 +140,7 @@ BOOST_AUTO_TEST_CASE(AsUniquePtrTest) { std::unique_ptr factory = NavigationPolicyFactory::make().add().asUniquePtr(); - auto policyBase = factory->build(volume); + auto policyBase = factory->build(gctx, volume, *logger); auto& policy = dynamic_cast&>(*policyBase); NavigationDelegate delegate; @@ -177,7 +184,8 @@ struct IsolatedConfig { int value; }; -auto makeCPolicy(const TrackingVolume& volume, IsolatedConfig config) { +auto makeCPolicy(const GeometryContext& /*gctx*/, const TrackingVolume& volume, + const Logger& /*logger*/, IsolatedConfig config) { // I can do arbitrary stuff here CPolicySpecialized::Config config2{.value = config.value}; return CPolicySpecialized(volume, config2); @@ -196,7 +204,7 @@ BOOST_AUTO_TEST_CASE(IsolatedFactory) { auto factory2 = NavigationPolicyFactory::make().add(makeCPolicy, config).add(); - auto policyBase = factory(volume); + auto policyBase = factory(gctx, volume, *logger); auto& policy = dynamic_cast>&>( *policyBase); From 646f5f885a8da31aee7ef0ff94a692a8b61350b3 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Sat, 12 Oct 2024 14:30:58 +0200 Subject: [PATCH 08/25] refactor: NavPolFactory captures all arguments by value Their lifetime needs to be controlled --- .../Acts/Geometry/NavigationPolicyFactory.hpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp index 3056cb10c77..1c1760b3f21 100644 --- a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp +++ b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp @@ -75,9 +75,9 @@ class NavigationPolicyFactoryImpl<> { Args...> && (std::is_copy_constructible_v && ...)) { - auto factory = [&](const GeometryContext& gctx, + auto factory = [=](const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger) { - return P{gctx, volume, logger, std::forward(args)...}; + return P{gctx, volume, logger, args...}; }; return NavigationPolicyFactoryImpl{ @@ -87,9 +87,9 @@ class NavigationPolicyFactoryImpl<> { template requires(NavigationPolicyIsolatedFactoryConcept) constexpr auto add(Fn&& fn, Args&&... args) { - auto factory = [&](const GeometryContext& gctx, + auto factory = [=](const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger) { - return fn(gctx, volume, logger, std::forward(args)...); + return fn(gctx, volume, logger, args...); }; return NavigationPolicyFactoryImpl{ @@ -115,9 +115,9 @@ class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { Args...> && (std::is_copy_constructible_v && ...)) { - auto factory = [&](const GeometryContext& gctx, + auto factory = [=](const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger) { - return P{gctx, volume, logger, std::forward(args)...}; + return P{gctx, volume, logger, args...}; }; return NavigationPolicyFactoryImpl{ @@ -128,9 +128,9 @@ class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { template requires(NavigationPolicyIsolatedFactoryConcept) constexpr auto add(Fn&& fn, Args&&... args) { - auto factory = [&](const GeometryContext& gctx, + auto factory = [=](const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger) { - return fn(gctx, volume, logger, std::forward(args)...); + return fn(gctx, volume, logger, args...); }; return NavigationPolicyFactoryImpl{ From 9e21a6646ea0495aa4c2d50ee66aae6d964e6cf2 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Sat, 12 Oct 2024 14:31:57 +0200 Subject: [PATCH 09/25] blueprint: Add SurfaceArrayNavPol stub --- .../SurfaceArrayNavigationPolicy.hpp | 54 +++++++++++++++++++ Core/src/Navigation/CMakeLists.txt | 5 +- .../SurfaceArrayNavigationPolicy.cpp | 32 +++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp create mode 100644 Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp diff --git a/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp b/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp new file mode 100644 index 00000000000..6d7d2b643a6 --- /dev/null +++ b/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp @@ -0,0 +1,54 @@ +// 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/Geometry/SurfaceArrayCreator.hpp" +#include "Acts/Navigation/INavigationPolicy.hpp" +#include "Acts/Surfaces/SurfaceArray.hpp" + +#pragma once + +namespace Acts { + +class SurfaceArrayNavigationPolicy : public INavigationPolicy { + public: + enum class LayerType { Cylinder, Disc, Plane }; + + struct Config { + SurfaceArrayCreator::Config surfaceArrayConfig{}; + LayerType layerType = LayerType::Cylinder; + }; + + explicit SurfaceArrayNavigationPolicy(const GeometryContext& gctx, + const TrackingVolume& volume, + const Logger& logger, Config config); + + void updateState(const NavigationArguments& args) const; + + void connect(NavigationDelegate& delegate) const override; + + friend std::ostream& operator<<(std::ostream& os, + const LayerType& layerType) { + switch (layerType) { + case LayerType::Cylinder: + os << "Cylinder"; + break; + case LayerType::Disc: + os << "Disc"; + break; + case LayerType::Plane: + os << "Plane"; + break; + } + return os; + } + + private: + std::unique_ptr m_surfaceArray{}; +}; + +} // namespace Acts diff --git a/Core/src/Navigation/CMakeLists.txt b/Core/src/Navigation/CMakeLists.txt index 678db0e4ef7..afd09ce5484 100644 --- a/Core/src/Navigation/CMakeLists.txt +++ b/Core/src/Navigation/CMakeLists.txt @@ -1,4 +1,7 @@ target_sources( ActsCore - PRIVATE NavigationStream.cpp TryAllNavigationPolicies.cpp + PRIVATE + NavigationStream.cpp + TryAllNavigationPolicies.cpp + SurfaceArrayNavigationPolicy.cpp ) diff --git a/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp b/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp new file mode 100644 index 00000000000..ddb181d6244 --- /dev/null +++ b/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp @@ -0,0 +1,32 @@ +// 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/Navigation/SurfaceArrayNavigationPolicy.hpp" + +#include "Acts/Geometry/TrackingVolume.hpp" + +namespace Acts { + +SurfaceArrayNavigationPolicy::SurfaceArrayNavigationPolicy( + const GeometryContext& gctx, const TrackingVolume& volume, + const Logger& logger, Config config) { + ACTS_VERBOSE("Constructing SurfaceArrayNavigationPolicy for volume " + << volume.volumeName()); + ACTS_VERBOSE("Layer type is " << config.layerType); + // collect sensitive surfaces from the volume + // std::vector>; +} + +void SurfaceArrayNavigationPolicy::updateState( + const NavigationArguments& args) const {} + +void SurfaceArrayNavigationPolicy::connect(NavigationDelegate& delegate) const { + connectDefault(delegate); +} + +} // namespace Acts From c0f417e2f112dbfaf45d5f6de0e4b9b607051e11 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Tue, 15 Oct 2024 13:59:51 +0200 Subject: [PATCH 10/25] feat: Python bindings for nav policy factory --- .../Navigation/TryAllNavigationPolicies.cpp | 14 +- Examples/Python/CMakeLists.txt | 1 + Examples/Python/src/ModuleEntry.cpp | 2 + Examples/Python/src/Navigation.cpp | 175 ++++++++++++++++++ Examples/Python/tests/test_navigation.py | 41 ++++ 5 files changed, 229 insertions(+), 4 deletions(-) create mode 100644 Examples/Python/src/Navigation.cpp create mode 100644 Examples/Python/tests/test_navigation.py diff --git a/Core/src/Navigation/TryAllNavigationPolicies.cpp b/Core/src/Navigation/TryAllNavigationPolicies.cpp index 1fd77bcc368..6b1730ad520 100644 --- a/Core/src/Navigation/TryAllNavigationPolicies.cpp +++ b/Core/src/Navigation/TryAllNavigationPolicies.cpp @@ -15,8 +15,11 @@ namespace Acts { TryAllPortalNavigationPolicy::TryAllPortalNavigationPolicy( const GeometryContext& /*gctx*/, const TrackingVolume& volume, - const Logger& /*logger*/) - : m_volume(&volume) {} + const Logger& logger) + : m_volume(&volume) { + ACTS_VERBOSE("TryAllPortalNavigationPolicy created for volume " + << m_volume->volumeName()); +} void TryAllPortalNavigationPolicy::updateState( const NavigationArguments& args) const { @@ -33,8 +36,11 @@ void TryAllPortalNavigationPolicy::connect(NavigationDelegate& delegate) const { TryAllSurfaceNavigationPolicy::TryAllSurfaceNavigationPolicy( const GeometryContext& /*gctx*/, const TrackingVolume& volume, - const Logger& /*logger*/) - : m_volume(&volume) {} + const Logger& logger) + : m_volume(&volume) { + ACTS_VERBOSE("TryAllSurfaceNavigationPolicy created for volume " + << m_volume->volumeName()); +} void TryAllSurfaceNavigationPolicy::updateState( const NavigationArguments& args) const { diff --git a/Examples/Python/CMakeLists.txt b/Examples/Python/CMakeLists.txt index ff1e5901f43..5ceaf63e8a6 100644 --- a/Examples/Python/CMakeLists.txt +++ b/Examples/Python/CMakeLists.txt @@ -21,6 +21,7 @@ pybind11_add_module(ActsPythonBindings src/Output.cpp src/Input.cpp src/Propagation.cpp + src/Navigation.cpp src/Generators.cpp src/Obj.cpp src/TruthTracking.cpp diff --git a/Examples/Python/src/ModuleEntry.cpp b/Examples/Python/src/ModuleEntry.cpp index 1a0e27db907..a74d277f7a5 100644 --- a/Examples/Python/src/ModuleEntry.cpp +++ b/Examples/Python/src/ModuleEntry.cpp @@ -50,6 +50,7 @@ void addBinning(Context& ctx); void addEventData(Context& ctx); void addPropagation(Context& ctx); +void addNavigation(Context& ctx); void addGeometry(Context& ctx); void addGeometryBuildingGen1(Context& ctx); @@ -123,6 +124,7 @@ PYBIND11_MODULE(ActsPythonBindings, m) { addOutput(ctx); addPropagation(ctx); + addNavigation(ctx); addGeometryBuildingGen1(ctx); addGeometry(ctx); addExperimentalGeometry(ctx); diff --git a/Examples/Python/src/Navigation.cpp b/Examples/Python/src/Navigation.cpp new file mode 100644 index 00000000000..d29060af112 --- /dev/null +++ b/Examples/Python/src/Navigation.cpp @@ -0,0 +1,175 @@ +// 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/Geometry/CylinderVolumeBounds.hpp" +#include "Acts/Geometry/NavigationPolicyFactory.hpp" +#include "Acts/Geometry/TrackingVolume.hpp" +#include "Acts/Navigation/SurfaceArrayNavigationPolicy.hpp" +#include "Acts/Navigation/TryAllNavigationPolicies.hpp" +#include "Acts/Plugins/Python/Utilities.hpp" +#include "Acts/Utilities/Logger.hpp" +#include "Acts/Utilities/TypeTag.hpp" + +#include +#include +#include + +#include +#include +#include + +namespace py = pybind11; +using namespace pybind11::literals; + +namespace Acts::Python { + +struct AnyNavigationPolicyFactory : public Acts::NavigationPolicyFactory { + virtual std::unique_ptr add( + TypeTag /*type*/) = 0; + + virtual std::unique_ptr add( + TypeTag /*type*/) = 0; + + virtual std::unique_ptr add( + TypeTag /*type*/, + SurfaceArrayNavigationPolicy::Config config) = 0; +}; + +template , + typename... Policies> +struct NavigationPolicyFactoryT : public AnyNavigationPolicyFactory { + NavigationPolicyFactoryT(Factory impl) + requires(sizeof...(Policies) > 0) + : m_impl(std::move(impl)) {} + + NavigationPolicyFactoryT() + requires(sizeof...(Policies) == 0) + : m_impl{} {} + + std::unique_ptr add( + TypeTag /*type*/) override { + return add(); + } + + std::unique_ptr add( + TypeTag /*type*/) override { + return add(); + } + + std::unique_ptr add( + TypeTag /*type*/, + SurfaceArrayNavigationPolicy::Config config) override { + return add(std::move(config)); + } + + std::unique_ptr build( + const GeometryContext& gctx, const TrackingVolume& volume, + const Logger& logger) const override { + if constexpr (sizeof...(Policies) > 0) { + return m_impl.build(gctx, volume, logger); + } else { + throw std::runtime_error("No policies added to the factory"); + } + } + + private: + template + std::unique_ptr add(Args&&... args) { + if constexpr (!((std::is_same_v || ...))) { + auto impl = m_impl.template add(std::forward(args)...); + return std::make_unique< + NavigationPolicyFactoryT>( + std::move(impl)); + } else { + throw std::invalid_argument("Policy already added to the factory"); + } + } + + Factory m_impl; +}; + +class NavigationPolicyFactory : public Acts::NavigationPolicyFactory { + public: + // This overload is for all the navigation policies that don't have extra + // arguments + NavigationPolicyFactory& addNoArguments(const py::object& cls) { + auto m = py::module_::import("acts"); + if (py::object o = m.attr("TryAllPortalNavigationPolicy"); cls.is(o)) { + m_impl = m_impl->add(Type); + } else if (o = m.attr("TryAllSurfaceNavigationPolicy"); cls.is(o)) { + m_impl = m_impl->add(Type); + } else { + } + return *this; + } + + NavigationPolicyFactory& addSurfaceArray( + const py::object& /*cls*/, + const SurfaceArrayNavigationPolicy::Config& config) { + m_impl = m_impl->add(Type, config); + return *this; + } + + std::unique_ptr build( + const GeometryContext& gctx, const TrackingVolume& volume, + const Logger& logger) const override { + return m_impl->build(gctx, volume, logger); + } + + private: + std::unique_ptr m_impl = + std::make_unique>(); +}; + +void addNavigation(Context& ctx) { + auto m = ctx.get("main"); + + py::class_>( + m, "_NavigationPolicyFactory"); + + py::class_(m, "TryAllPortalNavigationPolicy"); + py::class_(m, "TryAllSurfaceNavigationPolicy"); + + py::class_>( + m, "NavigationPolicyFactory") + .def(py::init<>()) + .def("add", &NavigationPolicyFactory::addNoArguments) + .def("add", &NavigationPolicyFactory::addSurfaceArray) + .def("_buildTest", [](NavigationPolicyFactory& self) { + auto vol1 = std::make_shared( + Transform3::Identity(), + std::make_shared(30, 40, 100)); + vol1->setVolumeName("TestVolume"); + + std::unique_ptr result = + self.build(GeometryContext{}, *vol1, + *getDefaultLogger("Test", Logging::VERBOSE)); + }); + + { + auto saPolicy = py::class_( + m, "SurfaceArrayNavigationPolicy"); + + using LayerType = SurfaceArrayNavigationPolicy::LayerType; + py::enum_(saPolicy, "LayerType") + .value("Cylinder", LayerType::Cylinder) + .value("Disc", LayerType::Disc) + .value("Plane", LayerType::Plane); + + using Config = SurfaceArrayNavigationPolicy::Config; + auto c = py::class_(saPolicy, "Config").def(py::init<>()); + ACTS_PYTHON_STRUCT_BEGIN(c, Config); + ACTS_PYTHON_MEMBER(surfaceArrayConfig); + ACTS_PYTHON_MEMBER(layerType); + ACTS_PYTHON_STRUCT_END(); + } +} + +} // namespace Acts::Python diff --git a/Examples/Python/tests/test_navigation.py b/Examples/Python/tests/test_navigation.py new file mode 100644 index 00000000000..c6f755f69b1 --- /dev/null +++ b/Examples/Python/tests/test_navigation.py @@ -0,0 +1,41 @@ +import pytest + +import acts + +import acts.examples + + +def test_navigation_policy_factory(): + + policy = ( + acts.NavigationPolicyFactory() + .add(acts.TryAllPortalNavigationPolicy) + .add(acts.TryAllSurfaceNavigationPolicy) + ) + + policy._buildTest() + + policy = ( + acts.NavigationPolicyFactory() + .add(acts.TryAllPortalNavigationPolicy) + .add(acts.TryAllSurfaceNavigationPolicy) + ) + + policy._buildTest() + + +def test_navigation_policy_factory_build_empty(): + policy = acts.NavigationPolicyFactory() + + with pytest.raises(RuntimeError): + policy._buildTest() + + +def test_navigation_policy_factory_add_multiple(): + with pytest.raises(ValueError): + ( + acts.NavigationPolicyFactory() + .add(acts.TryAllPortalNavigationPolicy) + .add(acts.TryAllSurfaceNavigationPolicy) + .add(acts.TryAllPortalNavigationPolicy) + ) From 1839c8dce9afd10bf42284aa61fde7cda46503ae Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Tue, 15 Oct 2024 14:00:13 +0200 Subject: [PATCH 11/25] test: Add SrfArrNavPol to python factory api test --- Examples/Python/tests/test_navigation.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Examples/Python/tests/test_navigation.py b/Examples/Python/tests/test_navigation.py index c6f755f69b1..6a74af590bc 100644 --- a/Examples/Python/tests/test_navigation.py +++ b/Examples/Python/tests/test_navigation.py @@ -11,6 +11,12 @@ def test_navigation_policy_factory(): acts.NavigationPolicyFactory() .add(acts.TryAllPortalNavigationPolicy) .add(acts.TryAllSurfaceNavigationPolicy) + .add( + acts.SurfaceArrayNavigationPolicy, + acts.SurfaceArrayNavigationPolicy.Config( + layerType=acts.SurfaceArrayNavigationPolicy.LayerType.Plane + ), + ) ) policy._buildTest() From ecf1fe19a2396ecbba4e55be09dcb4afa1b1ecc9 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Mon, 14 Oct 2024 16:38:15 +0200 Subject: [PATCH 12/25] refactor: Add logger to `NavigationArguments` --- Core/include/Acts/Navigation/NavigationDelegate.hpp | 3 +++ Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp | 5 ++++- Core/src/Navigation/TryAllNavigationPolicies.cpp | 4 ++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Core/include/Acts/Navigation/NavigationDelegate.hpp b/Core/include/Acts/Navigation/NavigationDelegate.hpp index 4a9b08cb3fd..6ae520c9215 100644 --- a/Core/include/Acts/Navigation/NavigationDelegate.hpp +++ b/Core/include/Acts/Navigation/NavigationDelegate.hpp @@ -15,6 +15,7 @@ namespace Acts { class NavigationStream; +class Logger; struct NavigationArguments { NavigationStream& main; @@ -22,6 +23,8 @@ struct NavigationArguments { Vector3 direction; BoundaryTolerance tolerance = BoundaryTolerance::None(); + + const Logger& logger; }; using NavigationDelegate = OwningDelegate; diff --git a/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp b/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp index ddb181d6244..ca80ec87126 100644 --- a/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp +++ b/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp @@ -23,7 +23,10 @@ SurfaceArrayNavigationPolicy::SurfaceArrayNavigationPolicy( } void SurfaceArrayNavigationPolicy::updateState( - const NavigationArguments& args) const {} + const NavigationArguments& args) const { + const Logger& logger = args.logger; + ACTS_VERBOSE("SrfArrNavPol (volume=" << m_volume.volumeName() << ")"); +} void SurfaceArrayNavigationPolicy::connect(NavigationDelegate& delegate) const { connectDefault(delegate); diff --git a/Core/src/Navigation/TryAllNavigationPolicies.cpp b/Core/src/Navigation/TryAllNavigationPolicies.cpp index 6b1730ad520..b580827b04c 100644 --- a/Core/src/Navigation/TryAllNavigationPolicies.cpp +++ b/Core/src/Navigation/TryAllNavigationPolicies.cpp @@ -23,6 +23,8 @@ TryAllPortalNavigationPolicy::TryAllPortalNavigationPolicy( void TryAllPortalNavigationPolicy::updateState( const NavigationArguments& args) const { + const Logger& logger = args.logger; + ACTS_VERBOSE("TryAllPortalNavigationPolicy"); assert(m_volume != nullptr); for (const auto& portal : m_volume->portals()) { @@ -44,6 +46,8 @@ TryAllSurfaceNavigationPolicy::TryAllSurfaceNavigationPolicy( void TryAllSurfaceNavigationPolicy::updateState( const NavigationArguments& args) const { + const Logger& logger = args.logger; + ACTS_VERBOSE("TryAllSurfaceNavigationPolicy"); assert(m_volume != nullptr); for (const auto& surface : m_volume->surfaces()) { From e4b0e7c498ba3ae48290219bba8693bc51aef3b2 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Mon, 14 Oct 2024 16:39:34 +0200 Subject: [PATCH 13/25] refactor: SurfaceArrayNavigationPolicy basic working version --- .../SurfaceArrayNavigationPolicy.hpp | 3 +- .../SurfaceArrayNavigationPolicy.cpp | 54 +++++++++++++++++-- Examples/Python/src/Navigation.cpp | 5 +- Examples/Python/tests/test_navigation.py | 11 ++-- 4 files changed, 61 insertions(+), 12 deletions(-) diff --git a/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp b/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp index 6d7d2b643a6..a18ded6e329 100644 --- a/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp @@ -19,8 +19,8 @@ class SurfaceArrayNavigationPolicy : public INavigationPolicy { enum class LayerType { Cylinder, Disc, Plane }; struct Config { - SurfaceArrayCreator::Config surfaceArrayConfig{}; LayerType layerType = LayerType::Cylinder; + std::pair bins; }; explicit SurfaceArrayNavigationPolicy(const GeometryContext& gctx, @@ -49,6 +49,7 @@ class SurfaceArrayNavigationPolicy : public INavigationPolicy { private: std::unique_ptr m_surfaceArray{}; + const TrackingVolume& m_volume; }; } // namespace Acts diff --git a/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp b/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp index ca80ec87126..087019e6e3d 100644 --- a/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp +++ b/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp @@ -9,23 +9,69 @@ #include "Acts/Navigation/SurfaceArrayNavigationPolicy.hpp" #include "Acts/Geometry/TrackingVolume.hpp" +#include "Acts/Navigation/NavigationStream.hpp" +#include "Acts/Utilities/BinningType.hpp" + +#include namespace Acts { SurfaceArrayNavigationPolicy::SurfaceArrayNavigationPolicy( const GeometryContext& gctx, const TrackingVolume& volume, - const Logger& logger, Config config) { + const Logger& logger, Config config) + : m_volume(volume) { ACTS_VERBOSE("Constructing SurfaceArrayNavigationPolicy for volume " << volume.volumeName()); - ACTS_VERBOSE("Layer type is " << config.layerType); - // collect sensitive surfaces from the volume - // std::vector>; + ACTS_VERBOSE("~> Layer type is " << config.layerType); + ACTS_VERBOSE("~> bins: " << config.bins.first << " x " << config.bins.second); + + SurfaceArrayCreator::Config sacConfig; + SurfaceArrayCreator sac{sacConfig, logger.clone("SrfArrCrtr")}; + + std::vector> surfaces; + // @TODO: Fill only sensitives + surfaces.reserve(volume.surfaces().size()); + for (const auto& surface : volume.surfaces()) { + surfaces.push_back(surface.getSharedPtr()); + } + + if (config.layerType == LayerType::Disc) { + auto [binsR, binsPhi] = config.bins; + m_surfaceArray = + sac.surfaceArrayOnDisc(gctx, std::move(surfaces), binsPhi, binsR); + } else if (config.layerType == LayerType::Cylinder) { + auto [binsPhi, binsZ] = config.bins; + m_surfaceArray = + sac.surfaceArrayOnCylinder(gctx, std::move(surfaces), binsPhi, binsZ); + // m_surfaces = sac.createCylinderSurfaces(config.bins.first, + // config.bins.second); + } else if (config.layerType == LayerType::Plane) { + ACTS_ERROR("Plane layers are not yet supported"); + throw std::invalid_argument("Plane layers are not yet supported"); + } else { + throw std::invalid_argument("Unknown layer type"); + } + + if (!m_surfaceArray) { + ACTS_ERROR("Failed to create surface array"); + throw std::runtime_error("Failed to create surface array"); + } } void SurfaceArrayNavigationPolicy::updateState( const NavigationArguments& args) const { const Logger& logger = args.logger; ACTS_VERBOSE("SrfArrNavPol (volume=" << m_volume.volumeName() << ")"); + + ACTS_VERBOSE("Querying sensitive surfaces at " << args.position.transpose()); + const std::vector& sensitiveSurfaces = + m_surfaceArray->neighbors(args.position); + ACTS_VERBOSE("~> Surface array reports " << sensitiveSurfaces.size() + << " sensitive surfaces"); + + for (const auto* surface : sensitiveSurfaces) { + args.main.addSurfaceCandidate(*surface, args.tolerance); + }; } void SurfaceArrayNavigationPolicy::connect(NavigationDelegate& delegate) const { diff --git a/Examples/Python/src/Navigation.cpp b/Examples/Python/src/Navigation.cpp index d29060af112..abc9951e198 100644 --- a/Examples/Python/src/Navigation.cpp +++ b/Examples/Python/src/Navigation.cpp @@ -139,7 +139,8 @@ void addNavigation(Context& ctx) { py::class_>( m, "NavigationPolicyFactory") - .def(py::init<>()) + // only to mirror the C++ API + .def_static("make", []() { return NavigationPolicyFactory{}; }) .def("add", &NavigationPolicyFactory::addNoArguments) .def("add", &NavigationPolicyFactory::addSurfaceArray) .def("_buildTest", [](NavigationPolicyFactory& self) { @@ -166,8 +167,8 @@ void addNavigation(Context& ctx) { using Config = SurfaceArrayNavigationPolicy::Config; auto c = py::class_(saPolicy, "Config").def(py::init<>()); ACTS_PYTHON_STRUCT_BEGIN(c, Config); - ACTS_PYTHON_MEMBER(surfaceArrayConfig); ACTS_PYTHON_MEMBER(layerType); + ACTS_PYTHON_MEMBER(bins); ACTS_PYTHON_STRUCT_END(); } } diff --git a/Examples/Python/tests/test_navigation.py b/Examples/Python/tests/test_navigation.py index 6a74af590bc..9178cad6854 100644 --- a/Examples/Python/tests/test_navigation.py +++ b/Examples/Python/tests/test_navigation.py @@ -8,13 +8,14 @@ def test_navigation_policy_factory(): policy = ( - acts.NavigationPolicyFactory() + acts.NavigationPolicyFactory.make() .add(acts.TryAllPortalNavigationPolicy) .add(acts.TryAllSurfaceNavigationPolicy) .add( acts.SurfaceArrayNavigationPolicy, acts.SurfaceArrayNavigationPolicy.Config( - layerType=acts.SurfaceArrayNavigationPolicy.LayerType.Plane + layerType=acts.SurfaceArrayNavigationPolicy.LayerType.Plane, + bins=(10, 10), ), ) ) @@ -22,7 +23,7 @@ def test_navigation_policy_factory(): policy._buildTest() policy = ( - acts.NavigationPolicyFactory() + acts.NavigationPolicyFactory.make() .add(acts.TryAllPortalNavigationPolicy) .add(acts.TryAllSurfaceNavigationPolicy) ) @@ -31,7 +32,7 @@ def test_navigation_policy_factory(): def test_navigation_policy_factory_build_empty(): - policy = acts.NavigationPolicyFactory() + policy = acts.NavigationPolicyFactory.make() with pytest.raises(RuntimeError): policy._buildTest() @@ -40,7 +41,7 @@ def test_navigation_policy_factory_build_empty(): def test_navigation_policy_factory_add_multiple(): with pytest.raises(ValueError): ( - acts.NavigationPolicyFactory() + acts.NavigationPolicyFactory.make() .add(acts.TryAllPortalNavigationPolicy) .add(acts.TryAllSurfaceNavigationPolicy) .add(acts.TryAllPortalNavigationPolicy) From 0bd237ba3f78dcfb9e8ed9b51b12b7aba1a8430d Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 18 Oct 2024 11:25:25 +0200 Subject: [PATCH 14/25] fix: GCC compilation for policy factory --- .../Acts/Geometry/NavigationPolicyFactory.hpp | 23 +++++++++---------- .../Acts/Navigation/MultiNavigationPolicy.hpp | 1 + .../Navigation/TryAllNavigationPolicies.cpp | 1 + 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp index 1c1760b3f21..4af2b19d1ad 100644 --- a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp +++ b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp @@ -99,14 +99,6 @@ class NavigationPolicyFactoryImpl<> { template class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { - template - using policy_type_helper = decltype(MultiNavigationPolicy( - std::invoke(std::declval(), std::declval(), - std::declval(), - std::declval())...)); - - using policy_type = policy_type_helper; - public: template constexpr auto add(Args&&... args) @@ -144,11 +136,18 @@ class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { std::move(*this)); } - std::unique_ptr operator()(const GeometryContext& gctx, - const TrackingVolume& volume, - const Logger& logger) const { + auto operator()(const GeometryContext& gctx, const TrackingVolume& volume, + const Logger& logger) const { return std::apply( - [&](auto&&... factories) -> std::unique_ptr { + [&](auto&&... factories) { + // Deduce policy type explicitly here... + using policy_type = decltype(MultiNavigationPolicy{ + std::invoke(factories, std::declval(), + std::declval(), + std::declval())...}); + + // ... so we can create a unique_ptr of the concrete type here rather + // than the base. (`make_unique` can't do type deduction) return std::make_unique( std::invoke(factories, gctx, volume, logger)...); }, diff --git a/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp b/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp index 10c0b12e6e7..140b8f68b28 100644 --- a/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp @@ -15,6 +15,7 @@ namespace Acts { class MultiNavigationPolicyBase : public INavigationPolicy {}; template + requires(sizeof...(Policies) > 0) class MultiNavigationPolicy final : public MultiNavigationPolicyBase { public: MultiNavigationPolicy(Policies&&... policies) diff --git a/Core/src/Navigation/TryAllNavigationPolicies.cpp b/Core/src/Navigation/TryAllNavigationPolicies.cpp index b580827b04c..3c3d71f72dc 100644 --- a/Core/src/Navigation/TryAllNavigationPolicies.cpp +++ b/Core/src/Navigation/TryAllNavigationPolicies.cpp @@ -17,6 +17,7 @@ TryAllPortalNavigationPolicy::TryAllPortalNavigationPolicy( const GeometryContext& /*gctx*/, const TrackingVolume& volume, const Logger& logger) : m_volume(&volume) { + assert(m_volume != nullptr); ACTS_VERBOSE("TryAllPortalNavigationPolicy created for volume " << m_volume->volumeName()); } From fb74c4baa7f29b1f4267e712db6b0b223bf7c305 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 16 Oct 2024 17:28:49 +0200 Subject: [PATCH 15/25] fix after logger change --- .../Core/Navigation/NavigationPolicyTests.cpp | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp index ab58afa2a88..febec7031f9 100644 --- a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp +++ b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp @@ -78,8 +78,10 @@ BOOST_AUTO_TEST_CASE(DirectTest) { policy.connect(delegate); NavigationStream main; - delegate(NavigationArguments{ - .main = main, .position = Vector3::Zero(), .direction = Vector3::Zero()}); + delegate(NavigationArguments{.main = main, + .position = Vector3::Zero(), + .direction = Vector3::Zero(), + .logger = *logger}); BOOST_CHECK(std::get(policy.policies()).executed); BOOST_CHECK(std::get(policy.policies()).executed); @@ -110,8 +112,10 @@ BOOST_AUTO_TEST_CASE(FactoryTest) { policy.connect(delegate); NavigationStream main; - delegate(NavigationArguments{ - .main = main, .position = Vector3::Zero(), .direction = Vector3::Zero()}); + delegate(NavigationArguments{.main = main, + .position = Vector3::Zero(), + .direction = Vector3::Zero(), + .logger = *logger}); BOOST_CHECK(std::get(policy.policies()).executed); BOOST_CHECK(std::get(policy.policies()).executed); @@ -123,8 +127,10 @@ BOOST_AUTO_TEST_CASE(FactoryTest) { NavigationDelegate delegate2; policyBase2->connect(delegate2); - delegate2(NavigationArguments{ - .main = main, .position = Vector3::Zero(), .direction = Vector3::Zero()}); + delegate2(NavigationArguments{.main = main, + .position = Vector3::Zero(), + .direction = Vector3::Zero(), + .logger = *logger}); BOOST_CHECK(std::get(policy2.policies()).executed); BOOST_CHECK(std::get(policy2.policies()).executed); @@ -147,8 +153,10 @@ BOOST_AUTO_TEST_CASE(AsUniquePtrTest) { policyBase->connect(delegate); NavigationStream main; - delegate(NavigationArguments{ - .main = main, .position = Vector3::Zero(), .direction = Vector3::Zero()}); + delegate(NavigationArguments{.main = main, + .position = Vector3::Zero(), + .direction = Vector3::Zero(), + .logger = *logger}); BOOST_CHECK(std::get(policy.policies()).executed); } @@ -213,8 +221,10 @@ BOOST_AUTO_TEST_CASE(IsolatedFactory) { policyBase->connect(delegate); NavigationStream main; - delegate(NavigationArguments{ - .main = main, .position = Vector3::Zero(), .direction = Vector3::Zero()}); + delegate(NavigationArguments{.main = main, + .position = Vector3::Zero(), + .direction = Vector3::Zero(), + .logger = *logger}); BOOST_CHECK(std::get(policy.policies()).executed); BOOST_CHECK(std::get>(policy.policies()).executed); From 2e9d93b37c48eed317f20352bef0d8a994d9e018 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 18 Oct 2024 12:06:38 +0200 Subject: [PATCH 16/25] add doc comments --- Core/include/Acts/Geometry/TrackingVolume.hpp | 6 +++++ .../Acts/Navigation/INavigationPolicy.hpp | 20 ++++++++++++++++ .../Acts/Navigation/MultiNavigationPolicy.hpp | 20 ++++++++++++++++ .../Acts/Navigation/NavigationDelegate.hpp | 4 ++++ .../SurfaceArrayNavigationPolicy.hpp | 22 ++++++++++++++++++ .../Navigation/TryAllNavigationPolicies.hpp | 23 +++++++++++++++++-- .../SurfaceArrayNavigationPolicy.cpp | 4 +++- 7 files changed, 96 insertions(+), 3 deletions(-) diff --git a/Core/include/Acts/Geometry/TrackingVolume.hpp b/Core/include/Acts/Geometry/TrackingVolume.hpp index dab4ecef92c..7d99da344ff 100644 --- a/Core/include/Acts/Geometry/TrackingVolume.hpp +++ b/Core/include/Acts/Geometry/TrackingVolume.hpp @@ -496,8 +496,14 @@ class TrackingVolume : public Volume { const ViewConfig& portalViewConfig, const ViewConfig& sensitiveViewConfig) const; + /// Register a navigation policy with this volume. The argument can not be + /// nullptr. + /// @param policy is the navigation policy to be registered void setNavigationPolicy(std::unique_ptr policy); + /// Update the navigation state for this volume. Internally, this consults the + /// registered navigation policy, where the default is a noop. + /// @param args are the navigation arguments void updateNavigationState(const NavigationArguments& args) const; private: diff --git a/Core/include/Acts/Navigation/INavigationPolicy.hpp b/Core/include/Acts/Navigation/INavigationPolicy.hpp index 5584183f730..27a11c3a543 100644 --- a/Core/include/Acts/Navigation/INavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/INavigationPolicy.hpp @@ -18,6 +18,9 @@ namespace Acts { class TrackingVolume; class INavigationPolicy; +/// Concept for a navigation policy +/// This exists so `updateState` can be a non-virtual method and we still have a +/// way to enforce it exists. template concept NavigationPolicyConcept = requires { requires std::is_base_of_v; @@ -27,15 +30,32 @@ concept NavigationPolicyConcept = requires { }; }; +/// Base class for all navigation policies. The policy needs to be *connected* +/// to a delegate via a virtual method for it to become active. The update +/// method is not part of the class interface. The conventional `updateState` +/// method is only required for use with the navigation policy factory, +/// otherwise `connect` is free to connect any function. class INavigationPolicy { public: + /// Noop update function that is suitable as a default for default navigation + /// delegates. static void noopUpdate(const NavigationArguments& /*unused*/) {} + /// Virtual destructor so policies can be held through this base class. virtual ~INavigationPolicy() = default; + /// Connect a policy with a delegate (usually a member of a volume). + /// This method exists to allow a policy to ensure a non-virtual function is + /// registered with the delegate. + /// @param delegate The delegate to connect to virtual void connect(NavigationDelegate& delegate) const = 0; protected: + /// Internal helper function for derived classes that conform to the concept + /// and have a conventional `updateState` method. Mainly used to save some + /// boilerplate. + /// @tparam T The type of the navigation policy + /// @param delegate The delegate to connect to template void connectDefault(NavigationDelegate& delegate) const { // This cannot be a concept because we use it in CRTP below diff --git a/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp b/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp index 140b8f68b28..d733435d69f 100644 --- a/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp @@ -12,15 +12,31 @@ namespace Acts { +/// Base class for multi navigation policies class MultiNavigationPolicyBase : public INavigationPolicy {}; +/// Combined navigation policy that calls all contained other navigation +/// policies. This class only works with policies complying with +/// `NavigationPolicyConcept`, which means that they have a conventional +/// `updateState` method. +/// +/// Internally, this uses a delegate chain factory to produce an unrolled +/// delegate chain. +/// +/// @tparam Policies The navigation policies to be combined template requires(sizeof...(Policies) > 0) class MultiNavigationPolicy final : public MultiNavigationPolicyBase { public: + /// Constructor from a set of child policies. + /// @param policies The child policies MultiNavigationPolicy(Policies&&... policies) : m_policies{std::forward(policies)...} {} + /// Implementation of the connection to a navigation delegate. + /// It uses the delegate chain factory to produce a delegate chain and stores + /// that chain in the owning navigation delegate. + /// @param delegate The navigation delegate to connect to void connect(NavigationDelegate& delegate) const override { auto factory = add(DelegateChainBuilder{delegate}, std::index_sequence_for{}); @@ -28,9 +44,12 @@ class MultiNavigationPolicy final : public MultiNavigationPolicyBase { factory.store(delegate); } + /// Access the contained policies + /// @return The contained policies const std::tuple& policies() const { return m_policies; } private: + /// Internal helper to build the delegate chain template constexpr auto add( auto factory, @@ -38,6 +57,7 @@ class MultiNavigationPolicy final : public MultiNavigationPolicyBase { return add(factory); } + /// Internal helper to build the delegate chain template constexpr auto add(auto factory) const { using policy_type = std::tuple_element_t; diff --git a/Core/include/Acts/Navigation/NavigationDelegate.hpp b/Core/include/Acts/Navigation/NavigationDelegate.hpp index 6ae520c9215..4a9944ea1a1 100644 --- a/Core/include/Acts/Navigation/NavigationDelegate.hpp +++ b/Core/include/Acts/Navigation/NavigationDelegate.hpp @@ -17,6 +17,8 @@ namespace Acts { class NavigationStream; class Logger; +/// Struct that serves as the argument to the navigation delegate. +/// It is not supposed to be used as an lvalue. struct NavigationArguments { NavigationStream& main; Vector3 position; @@ -27,6 +29,8 @@ struct NavigationArguments { const Logger& logger; }; +/// Central alias for the navigation delegate. This type is owning to support +/// (type-erased) navigation delegate chains (i.e. multiple policies). using NavigationDelegate = OwningDelegate; } // namespace Acts diff --git a/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp b/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp index a18ded6e329..db788fce9c9 100644 --- a/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp @@ -14,23 +14,45 @@ namespace Acts { +/// A navigation policy that internally uses the Gen1 @c SurfaceArray class class SurfaceArrayNavigationPolicy : public INavigationPolicy { public: + /// Enum for configuring which type of surface array to produce. This affects + /// the projection that is used for creating the binning structure. enum class LayerType { Cylinder, Disc, Plane }; + /// Config struct to configure the surface array navigation struct Config { + /// The type of the layer LayerType layerType = LayerType::Cylinder; + /// The number of bins in the local directions. The interpretation depends + /// on the layer type. std::pair bins; }; + /// Main constructor, which internally creates the surface array acceleration + /// structure. + /// @note Expects that all relevant surfaces are registered with @p volume. + /// Only selects sensitive surfaces for the surface array. + /// @param gctx The geometry context + /// @param volume The *layer volume* to construct the surface array from + /// @param logger A logging instance + /// @param config The configuration for the surface array explicit SurfaceArrayNavigationPolicy(const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger, Config config); + /// Update the navigation state from the surface array + /// @param args The navigation arguments void updateState(const NavigationArguments& args) const; + /// Connect this policy with a navigation delegate + /// @param delegate The navigation delegate to connect to void connect(NavigationDelegate& delegate) const override; + /// Output stream operator for the contained layer type enum + /// @param os The output stream + /// @param layerType The layer type to print friend std::ostream& operator<<(std::ostream& os, const LayerType& layerType) { switch (layerType) { diff --git a/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp b/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp index 9e15ad35ceb..876d9210ad7 100644 --- a/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp +++ b/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp @@ -16,16 +16,24 @@ class TrackingVolume; class GeometryContext; class Logger; -// @NOTE: This implementation is PRELIMINARY! It is subject to change! - +/// Policy which adds **all** portals of the volume to the main navigation +/// stream class TryAllPortalNavigationPolicy final : public INavigationPolicy { public: + /// Constructor from a volume + /// @param gctx is the geometry context + /// @param volume is the volume to navigate + /// @param logger is the logger explicit TryAllPortalNavigationPolicy(const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger); + /// Update the navigation state with all volume portals. + /// @param args are the navigation arguments void updateState(const NavigationArguments& args) const; + /// Connect the policy to a navigation delegate + /// @param delegate is the navigation delegate void connect(NavigationDelegate& delegate) const override; private: @@ -34,14 +42,25 @@ class TryAllPortalNavigationPolicy final : public INavigationPolicy { static_assert(NavigationPolicyConcept); +/// Policy which adds **all** surfaces of the volume to the main navigation +/// @note This does **not** include portal surfaces, but does not make any distinction +/// between sensitive and other surfaces in the volume. class TryAllSurfaceNavigationPolicy final : public INavigationPolicy { public: + /// Constructor from a volume + /// @param gctx is the geometry context + /// @param volume is the volume to navigate + /// @param logger is the logger explicit TryAllSurfaceNavigationPolicy(const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger); + /// Update the navigation state with all volume surfaces. + /// @param args are the navigation arguments void updateState(const NavigationArguments& args) const; + /// Connect the policy to a navigation delegate + /// @param delegate is the navigation delegate void connect(NavigationDelegate& delegate) const override; private: diff --git a/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp b/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp index 087019e6e3d..e2eb4d88eed 100644 --- a/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp +++ b/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp @@ -29,9 +29,11 @@ SurfaceArrayNavigationPolicy::SurfaceArrayNavigationPolicy( SurfaceArrayCreator sac{sacConfig, logger.clone("SrfArrCrtr")}; std::vector> surfaces; - // @TODO: Fill only sensitives surfaces.reserve(volume.surfaces().size()); for (const auto& surface : volume.surfaces()) { + if (surface.associatedDetectorElement() == nullptr) { + continue; + } surfaces.push_back(surface.getSharedPtr()); } From fe3ece5202c93997c457ffd2f4aab4c214190657 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 18 Oct 2024 15:14:04 +0200 Subject: [PATCH 17/25] make `add` functions only work on rvalues --- .../Acts/Geometry/NavigationPolicyFactory.hpp | 18 +++++++++++++----- Examples/Python/src/Navigation.cpp | 3 ++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp index 4af2b19d1ad..b5ab5ff7616 100644 --- a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp +++ b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp @@ -25,6 +25,16 @@ template class NavigationPolicyFactoryImpl; } +/// Base class for navigation policy factories. The factory can be assembled +/// iteratively by using `make` followed by a number of calls to the `add` +/// function of the helper type. Example: +/// +/// ```cpp +/// auto factory = NavigationPolicyFactory::make() +/// .add(arg1, arg2) +/// .add(/*no args*/) +/// .asUniquePtr(); +/// ``` class NavigationPolicyFactory { public: virtual ~NavigationPolicyFactory() = default; @@ -69,12 +79,11 @@ class NavigationPolicyFactoryImpl<> { // Arguments need to be copy constructible because the factory must be able to // execute multiple times. template - constexpr auto add(Args&&... args) requires(std::is_constructible_v && (std::is_copy_constructible_v && ...)) - { + constexpr auto add(Args&&... args) && { auto factory = [=](const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger) { return P{gctx, volume, logger, args...}; @@ -101,12 +110,11 @@ template class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { public: template - constexpr auto add(Args&&... args) requires(std::is_constructible_v && (std::is_copy_constructible_v && ...)) - { + constexpr auto add(Args&&... args) && { auto factory = [=](const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger) { return P{gctx, volume, logger, args...}; @@ -119,7 +127,7 @@ class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { template requires(NavigationPolicyIsolatedFactoryConcept) - constexpr auto add(Fn&& fn, Args&&... args) { + constexpr auto add(Fn&& fn, Args&&... args) && { auto factory = [=](const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger) { return fn(gctx, volume, logger, args...); diff --git a/Examples/Python/src/Navigation.cpp b/Examples/Python/src/Navigation.cpp index abc9951e198..57dd62b2ae1 100644 --- a/Examples/Python/src/Navigation.cpp +++ b/Examples/Python/src/Navigation.cpp @@ -81,7 +81,8 @@ struct NavigationPolicyFactoryT : public AnyNavigationPolicyFactory { template std::unique_ptr add(Args&&... args) { if constexpr (!((std::is_same_v || ...))) { - auto impl = m_impl.template add(std::forward(args)...); + auto impl = + std::move(m_impl).template add(std::forward(args)...); return std::make_unique< NavigationPolicyFactoryT>( std::move(impl)); From 3c9b37d3f35636e0c2c1a90194fa8d4863c4f2a4 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 18 Oct 2024 15:14:23 +0200 Subject: [PATCH 18/25] Add doc domments to factory --- .../Acts/Geometry/NavigationPolicyFactory.hpp | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp index b5ab5ff7616..4d9542af826 100644 --- a/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp +++ b/Core/include/Acts/Geometry/NavigationPolicyFactory.hpp @@ -76,8 +76,12 @@ class NavigationPolicyFactoryImpl<> { friend class NavigationPolicyFactoryImpl; NavigationPolicyFactoryImpl() = default; - // Arguments need to be copy constructible because the factory must be able to - // execute multiple times. + /// Create a factory with the specified policy added + /// @tparam P The policy type to add + /// @param args The arguments to pass to the policy constructor + /// @note Arguments need to be copy constructible because the factory must be + /// able to execute multiple times. + /// @return A new policy factory including the @c P policy. template requires(std::is_constructible_v { std::make_tuple(std::move(factory))}; } + /// Create a factory with a policy returned by a factory function + /// @tparam Fn The type of the function to construct the policy + /// @param args The arguments to pass to the policy factory + /// @note Arguments need to be copy constructible because the factory must be + /// able to execute multiple times. + /// @return A new policy factory including the function template requires(NavigationPolicyIsolatedFactoryConcept) constexpr auto add(Fn&& fn, Args&&... args) { @@ -109,6 +119,12 @@ class NavigationPolicyFactoryImpl<> { template class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { public: + /// Create a factory with the specified policy added + /// @tparam P The policy type to add + /// @param args The arguments to pass to the policy constructor + /// @note Arguments need to be copy constructible because the factory must be + /// able to execute multiple times. + /// @return A new policy factory including the @c P policy. template requires(std::is_constructible_v : public NavigationPolicyFactory { std::make_tuple(std::move(factory)))}; } + /// Create a factory with a policy returned by a factory function + /// @tparam Fn The type of the function to construct the policy + /// @param args The arguments to pass to the policy factory + /// @note Arguments need to be copy constructible because the factory must be + /// able to execute multiple times. + /// @return A new policy factory including the function template requires(NavigationPolicyIsolatedFactoryConcept) constexpr auto add(Fn&& fn, Args&&... args) && { @@ -138,12 +160,19 @@ class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { std::make_tuple(std::move(factory)))}; } + /// Move the factory into a unique pointer + /// @note Only callable on rvalue references + /// @return A unique pointer to the factory constexpr std::unique_ptr> asUniquePtr() && { return std::make_unique>( std::move(*this)); } + /// Construct a navigation policy using the factories + /// @param gctx The geometry context + /// @param volume The tracking volume + /// @param logger The logger auto operator()(const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger) const { return std::apply( @@ -162,6 +191,10 @@ class NavigationPolicyFactoryImpl : public NavigationPolicyFactory { m_factories); } + /// Construct a navigation policy using the factories + /// @param gctx The geometry context + /// @param volume The tracking volume + /// @param logger The logger std::unique_ptr build( const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger) const override { From bbdc5b00e6b7af901d77e1b7f1b82c68ef2fe10d Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Tue, 22 Oct 2024 17:19:40 +0200 Subject: [PATCH 19/25] updates after PR feedback --- Core/include/Acts/Geometry/TrackingVolume.hpp | 12 +++- .../Acts/Navigation/INavigationPolicy.hpp | 12 +++- .../Acts/Navigation/MultiNavigationPolicy.hpp | 3 +- .../Acts/Navigation/NavigationDelegate.hpp | 7 +- .../Acts/Navigation/NavigationStream.hpp | 10 +++ .../SurfaceArrayNavigationPolicy.hpp | 12 +++- .../Navigation/TryAllNavigationPolicies.hpp | 72 ------------------- .../Navigation/TryAllNavigationPolicy.hpp | 57 +++++++++++++++ Core/src/Geometry/TrackingVolume.cpp | 10 +-- Core/src/Navigation/CMakeLists.txt | 2 +- Core/src/Navigation/NavigationStream.cpp | 41 +++++++---- .../SurfaceArrayNavigationPolicy.cpp | 10 +-- .../Navigation/TryAllNavigationPolicies.cpp | 64 ----------------- .../src/Navigation/TryAllNavigationPolicy.cpp | 53 ++++++++++++++ Examples/Python/src/Navigation.cpp | 25 ++----- Examples/Python/tests/test_navigation.py | 14 ++-- .../Core/Navigation/NavigationPolicyTests.cpp | 59 ++++++++------- 17 files changed, 235 insertions(+), 228 deletions(-) delete mode 100644 Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp create mode 100644 Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp delete mode 100644 Core/src/Navigation/TryAllNavigationPolicies.cpp create mode 100644 Core/src/Navigation/TryAllNavigationPolicy.cpp diff --git a/Core/include/Acts/Geometry/TrackingVolume.hpp b/Core/include/Acts/Geometry/TrackingVolume.hpp index 7d99da344ff..f47f8b440fc 100644 --- a/Core/include/Acts/Geometry/TrackingVolume.hpp +++ b/Core/include/Acts/Geometry/TrackingVolume.hpp @@ -19,6 +19,7 @@ #include "Acts/Geometry/Volume.hpp" #include "Acts/Material/IVolumeMaterial.hpp" #include "Acts/Navigation/NavigationDelegate.hpp" +#include "Acts/Navigation/NavigationStream.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Surfaces/SurfaceArray.hpp" #include "Acts/Surfaces/SurfaceVisitorConcept.hpp" @@ -501,10 +502,15 @@ class TrackingVolume : public Volume { /// @param policy is the navigation policy to be registered void setNavigationPolicy(std::unique_ptr policy); - /// Update the navigation state for this volume. Internally, this consults the - /// registered navigation policy, where the default is a noop. + /// Populate the navigation stream with navigation candidates from this + /// volume. Internally, this consults the registered navigation policy, where + /// the default is a noop. /// @param args are the navigation arguments - void updateNavigationState(const NavigationArguments& args) const; + /// @param stream is the navigation stream to be updated + /// @param logger is the logger + void initializeNavigationCandidates(const NavigationArguments& args, + AppendOnlyNavigationStream& stream, + const Logger& logger) const; private: void connectDenseBoundarySurfaces( diff --git a/Core/include/Acts/Navigation/INavigationPolicy.hpp b/Core/include/Acts/Navigation/INavigationPolicy.hpp index 27a11c3a543..6794dbdea09 100644 --- a/Core/include/Acts/Navigation/INavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/INavigationPolicy.hpp @@ -9,6 +9,7 @@ #pragma once #include "Acts/Navigation/NavigationDelegate.hpp" +#include "Acts/Navigation/NavigationStream.hpp" #include "Acts/Utilities/DelegateChainBuilder.hpp" #include @@ -26,7 +27,9 @@ concept NavigationPolicyConcept = requires { requires std::is_base_of_v; // Has a conforming update method requires requires(T policy, const NavigationArguments& args) { - { policy.updateState(args) }; + policy.initializeCandidates(args, + std::declval(), + std::declval()); }; }; @@ -39,7 +42,9 @@ class INavigationPolicy { public: /// Noop update function that is suitable as a default for default navigation /// delegates. - static void noopUpdate(const NavigationArguments& /*unused*/) {} + static void noopInitializeCandidates(const NavigationArguments& /*unused*/, + AppendOnlyNavigationStream& /*unused*/, + const Logger& /*unused*/) {} /// Virtual destructor so policies can be held through this base class. virtual ~INavigationPolicy() = default; @@ -60,7 +65,8 @@ class INavigationPolicy { void connectDefault(NavigationDelegate& delegate) const { // This cannot be a concept because we use it in CRTP below const auto* self = static_cast(this); - DelegateChainBuilder{delegate}.add<&T::updateState>(self).store(delegate); + DelegateChainBuilder{delegate}.add<&T::initializeCandidates>(self).store( + delegate); } }; diff --git a/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp b/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp index d733435d69f..67c1af6bde4 100644 --- a/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp @@ -63,7 +63,8 @@ class MultiNavigationPolicy final : public MultiNavigationPolicyBase { using policy_type = std::tuple_element_t; auto* policy = &std::get(m_policies); - auto added = factory.template add<&policy_type::updateState>(policy); + auto added = + factory.template add<&policy_type::initializeCandidates>(policy); if constexpr (sizeof...(Is) > 0) { return add(added); diff --git a/Core/include/Acts/Navigation/NavigationDelegate.hpp b/Core/include/Acts/Navigation/NavigationDelegate.hpp index 4a9944ea1a1..8e243818d30 100644 --- a/Core/include/Acts/Navigation/NavigationDelegate.hpp +++ b/Core/include/Acts/Navigation/NavigationDelegate.hpp @@ -9,6 +9,7 @@ #pragma once #include "Acts/Definitions/Algebra.hpp" +#include "Acts/Navigation/NavigationStream.hpp" #include "Acts/Surfaces/BoundaryTolerance.hpp" #include "Acts/Utilities/Delegate.hpp" @@ -20,17 +21,15 @@ class Logger; /// Struct that serves as the argument to the navigation delegate. /// It is not supposed to be used as an lvalue. struct NavigationArguments { - NavigationStream& main; Vector3 position; Vector3 direction; BoundaryTolerance tolerance = BoundaryTolerance::None(); - - const Logger& logger; }; /// Central alias for the navigation delegate. This type is owning to support /// (type-erased) navigation delegate chains (i.e. multiple policies). -using NavigationDelegate = OwningDelegate; +using NavigationDelegate = OwningDelegate; } // namespace Acts diff --git a/Core/include/Acts/Navigation/NavigationStream.hpp b/Core/include/Acts/Navigation/NavigationStream.hpp index 7e9c51dc7f9..97e154214d7 100644 --- a/Core/include/Acts/Navigation/NavigationStream.hpp +++ b/Core/include/Acts/Navigation/NavigationStream.hpp @@ -179,4 +179,14 @@ class NavigationStream { std::size_t m_currentIndex = 0u; }; +struct AppendOnlyNavigationStream { + explicit AppendOnlyNavigationStream(NavigationStream& stream); + void addSurfaceCandidate(const Surface& surface, + const BoundaryTolerance& bTolerance); + void addPortalCandidate(const Portal& portal); + + private: + NavigationStream* m_stream; +}; + } // namespace Acts diff --git a/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp b/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp index db788fce9c9..9143cf0087d 100644 --- a/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/SurfaceArrayNavigationPolicy.hpp @@ -6,14 +6,14 @@ // 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/Geometry/SurfaceArrayCreator.hpp" #include "Acts/Navigation/INavigationPolicy.hpp" -#include "Acts/Surfaces/SurfaceArray.hpp" #pragma once namespace Acts { +class SurfaceArray; + /// A navigation policy that internally uses the Gen1 @c SurfaceArray class class SurfaceArrayNavigationPolicy : public INavigationPolicy { public: @@ -44,7 +44,11 @@ class SurfaceArrayNavigationPolicy : public INavigationPolicy { /// Update the navigation state from the surface array /// @param args The navigation arguments - void updateState(const NavigationArguments& args) const; + /// @param stream The navigation stream to update + /// @param logger The logger + void initializeCandidates(const NavigationArguments& args, + AppendOnlyNavigationStream& stream, + const Logger& logger) const; /// Connect this policy with a navigation delegate /// @param delegate The navigation delegate to connect to @@ -74,4 +78,6 @@ class SurfaceArrayNavigationPolicy : public INavigationPolicy { const TrackingVolume& m_volume; }; +static_assert(NavigationPolicyConcept); + } // namespace Acts diff --git a/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp b/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp deleted file mode 100644 index 876d9210ad7..00000000000 --- a/Core/include/Acts/Navigation/TryAllNavigationPolicies.hpp +++ /dev/null @@ -1,72 +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 "Acts/Navigation/INavigationPolicy.hpp" - -namespace Acts { - -class TrackingVolume; -class GeometryContext; -class Logger; - -/// Policy which adds **all** portals of the volume to the main navigation -/// stream -class TryAllPortalNavigationPolicy final : public INavigationPolicy { - public: - /// Constructor from a volume - /// @param gctx is the geometry context - /// @param volume is the volume to navigate - /// @param logger is the logger - explicit TryAllPortalNavigationPolicy(const GeometryContext& gctx, - const TrackingVolume& volume, - const Logger& logger); - - /// Update the navigation state with all volume portals. - /// @param args are the navigation arguments - void updateState(const NavigationArguments& args) const; - - /// Connect the policy to a navigation delegate - /// @param delegate is the navigation delegate - void connect(NavigationDelegate& delegate) const override; - - private: - const TrackingVolume* m_volume; -}; - -static_assert(NavigationPolicyConcept); - -/// Policy which adds **all** surfaces of the volume to the main navigation -/// @note This does **not** include portal surfaces, but does not make any distinction -/// between sensitive and other surfaces in the volume. -class TryAllSurfaceNavigationPolicy final : public INavigationPolicy { - public: - /// Constructor from a volume - /// @param gctx is the geometry context - /// @param volume is the volume to navigate - /// @param logger is the logger - explicit TryAllSurfaceNavigationPolicy(const GeometryContext& gctx, - const TrackingVolume& volume, - const Logger& logger); - - /// Update the navigation state with all volume surfaces. - /// @param args are the navigation arguments - void updateState(const NavigationArguments& args) const; - - /// Connect the policy to a navigation delegate - /// @param delegate is the navigation delegate - void connect(NavigationDelegate& delegate) const override; - - private: - const TrackingVolume* m_volume; -}; - -static_assert(NavigationPolicyConcept); - -} // namespace Acts diff --git a/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp b/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp new file mode 100644 index 00000000000..bef9dab6e0d --- /dev/null +++ b/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp @@ -0,0 +1,57 @@ +// 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/Navigation/INavigationPolicy.hpp" +#include "Acts/Navigation/NavigationStream.hpp" + +namespace Acts { + +class TrackingVolume; +class GeometryContext; +class Logger; + +/// Policy which adds **all** candidates of the configured type to the +/// stream +class TryAllNavigationPolicy final : public INavigationPolicy { + public: + struct Config { + bool portals = true; + bool sensitives = true; + }; + + /// Constructor from a volume + /// @param config The configuration for the policy + /// @param gctx is the geometry context + /// @param volume is the volume to navigate + /// @param logger is the logger + TryAllNavigationPolicy(const Config& config, const GeometryContext& gctx, + const TrackingVolume& volume, const Logger& logger); + + TryAllNavigationPolicy(const GeometryContext& gctx, + const TrackingVolume& volume, const Logger& logger); + + /// Update the navigation state with all volume portals. + /// @param args are the navigation arguments + void initializeCandidates(const NavigationArguments& args, + AppendOnlyNavigationStream& stream, + const Logger& logger) const; + + /// Connect the policy to a navigation delegate + /// @param delegate is the navigation delegate + void connect(NavigationDelegate& delegate) const override; + + private: + Config m_cfg; + const TrackingVolume* m_volume; +}; + +static_assert(NavigationPolicyConcept); + +} // namespace Acts diff --git a/Core/src/Geometry/TrackingVolume.cpp b/Core/src/Geometry/TrackingVolume.cpp index 8fc7366abdb..611904537f0 100644 --- a/Core/src/Geometry/TrackingVolume.cpp +++ b/Core/src/Geometry/TrackingVolume.cpp @@ -17,6 +17,7 @@ #include "Acts/Material/IVolumeMaterial.hpp" #include "Acts/Material/ProtoVolumeMaterial.hpp" #include "Acts/Navigation/INavigationPolicy.hpp" +#include "Acts/Navigation/NavigationStream.hpp" #include "Acts/Propagator/Navigator.hpp" #include "Acts/Surfaces/RegularSurface.hpp" #include "Acts/Surfaces/Surface.hpp" @@ -52,7 +53,7 @@ TrackingVolume::TrackingVolume( connectDenseBoundarySurfaces(denseVolumeVector); DelegateChainBuilder{m_navigationDelegate} - .add<&INavigationPolicy::noopUpdate>() + .add<&INavigationPolicy::noopInitializeCandidates>() .store(m_navigationDelegate); } @@ -754,9 +755,10 @@ void TrackingVolume::setNavigationPolicy( m_navigationPolicy->connect(m_navigationDelegate); } -void TrackingVolume::updateNavigationState( - const NavigationArguments& args) const { - m_navigationDelegate(args); +void TrackingVolume::initializeNavigationCandidates( + const NavigationArguments& args, AppendOnlyNavigationStream& stream, + const Logger& logger) const { + m_navigationDelegate(args, stream, logger); } } // namespace Acts diff --git a/Core/src/Navigation/CMakeLists.txt b/Core/src/Navigation/CMakeLists.txt index afd09ce5484..72bbf65b853 100644 --- a/Core/src/Navigation/CMakeLists.txt +++ b/Core/src/Navigation/CMakeLists.txt @@ -2,6 +2,6 @@ target_sources( ActsCore PRIVATE NavigationStream.cpp - TryAllNavigationPolicies.cpp + TryAllNavigationPolicy.cpp SurfaceArrayNavigationPolicy.cpp ) diff --git a/Core/src/Navigation/NavigationStream.cpp b/Core/src/Navigation/NavigationStream.cpp index 62ec7eb11eb..c36eed12bbe 100644 --- a/Core/src/Navigation/NavigationStream.cpp +++ b/Core/src/Navigation/NavigationStream.cpp @@ -13,10 +13,12 @@ #include -bool Acts::NavigationStream::initialize(const GeometryContext& gctx, - const QueryPoint& queryPoint, - const BoundaryTolerance& cTolerance, - ActsScalar onSurfaceTolerance) { +namespace Acts { + +bool NavigationStream::initialize(const GeometryContext& gctx, + const QueryPoint& queryPoint, + const BoundaryTolerance& cTolerance, + ActsScalar onSurfaceTolerance) { // Position and direction from the query point const Vector3& position = queryPoint.position; const Vector3& direction = queryPoint.direction; @@ -91,9 +93,9 @@ bool Acts::NavigationStream::initialize(const GeometryContext& gctx, return true; } -bool Acts::NavigationStream::update(const GeometryContext& gctx, - const QueryPoint& queryPoint, - ActsScalar onSurfaceTolerance) { +bool NavigationStream::update(const GeometryContext& gctx, + const QueryPoint& queryPoint, + ActsScalar onSurfaceTolerance) { // Loop over the (currently valid) candidates and update for (; m_currentIndex < m_candidates.size(); ++m_currentIndex) { // Get the candidate, and resolve the tuple @@ -121,14 +123,14 @@ bool Acts::NavigationStream::update(const GeometryContext& gctx, return false; } -void Acts::NavigationStream::addSurfaceCandidate( +void NavigationStream::addSurfaceCandidate( const Surface& surface, const BoundaryTolerance& bTolerance) { m_candidates.emplace_back( Candidate{.intersection = ObjectIntersection::invalid(&surface), .bTolerance = bTolerance}); } -void Acts::NavigationStream::addSurfaceCandidates( +void NavigationStream::addSurfaceCandidates( std::span surfaces, const BoundaryTolerance& bTolerance) { std::ranges::for_each(surfaces, [&](const auto* surface) { m_candidates.emplace_back( @@ -137,22 +139,21 @@ void Acts::NavigationStream::addSurfaceCandidates( }); } -void Acts::NavigationStream::addPortalCandidate( - const Experimental::Portal& portal) { +void NavigationStream::addPortalCandidate(const Experimental::Portal& portal) { m_candidates.emplace_back(Candidate{ .intersection = ObjectIntersection::invalid(&portal.surface()), .gen2Portal = &portal, .bTolerance = BoundaryTolerance::None()}); } -void Acts::NavigationStream::addPortalCandidate(const Portal& portal) { +void NavigationStream::addPortalCandidate(const Portal& portal) { m_candidates.emplace_back(Candidate{ .intersection = ObjectIntersection::invalid(&portal.surface()), .portal = &portal, .bTolerance = BoundaryTolerance::None()}); } -void Acts::NavigationStream::addPortalCandidates( +void NavigationStream::addPortalCandidates( std::span portals) { std::ranges::for_each(portals, [&](const auto& portal) { m_candidates.emplace_back(Candidate{ @@ -162,3 +163,17 @@ void Acts::NavigationStream::addPortalCandidates( .bTolerance = BoundaryTolerance::None()}); }); } + +AppendOnlyNavigationStream::AppendOnlyNavigationStream(NavigationStream& stream) + : m_stream{&stream} {} + +void AppendOnlyNavigationStream::addPortalCandidate(const Portal& portal) { + m_stream->addPortalCandidate(portal); +} + +void AppendOnlyNavigationStream::addSurfaceCandidate( + const Surface& surface, const BoundaryTolerance& bTolerance) { + m_stream->addSurfaceCandidate(surface, bTolerance); +} + +} // namespace Acts diff --git a/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp b/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp index e2eb4d88eed..f40621fdd06 100644 --- a/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp +++ b/Core/src/Navigation/SurfaceArrayNavigationPolicy.cpp @@ -8,9 +8,9 @@ #include "Acts/Navigation/SurfaceArrayNavigationPolicy.hpp" +#include "Acts/Geometry/SurfaceArrayCreator.hpp" #include "Acts/Geometry/TrackingVolume.hpp" #include "Acts/Navigation/NavigationStream.hpp" -#include "Acts/Utilities/BinningType.hpp" #include @@ -60,9 +60,9 @@ SurfaceArrayNavigationPolicy::SurfaceArrayNavigationPolicy( } } -void SurfaceArrayNavigationPolicy::updateState( - const NavigationArguments& args) const { - const Logger& logger = args.logger; +void SurfaceArrayNavigationPolicy::initializeCandidates( + const NavigationArguments& args, AppendOnlyNavigationStream& stream, + const Logger& logger) const { ACTS_VERBOSE("SrfArrNavPol (volume=" << m_volume.volumeName() << ")"); ACTS_VERBOSE("Querying sensitive surfaces at " << args.position.transpose()); @@ -72,7 +72,7 @@ void SurfaceArrayNavigationPolicy::updateState( << " sensitive surfaces"); for (const auto* surface : sensitiveSurfaces) { - args.main.addSurfaceCandidate(*surface, args.tolerance); + stream.addSurfaceCandidate(*surface, args.tolerance); }; } diff --git a/Core/src/Navigation/TryAllNavigationPolicies.cpp b/Core/src/Navigation/TryAllNavigationPolicies.cpp deleted file mode 100644 index 3c3d71f72dc..00000000000 --- a/Core/src/Navigation/TryAllNavigationPolicies.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 "Acts/Navigation/TryAllNavigationPolicies.hpp" - -#include "Acts/Geometry/TrackingVolume.hpp" -#include "Acts/Navigation/NavigationStream.hpp" - -namespace Acts { - -TryAllPortalNavigationPolicy::TryAllPortalNavigationPolicy( - const GeometryContext& /*gctx*/, const TrackingVolume& volume, - const Logger& logger) - : m_volume(&volume) { - assert(m_volume != nullptr); - ACTS_VERBOSE("TryAllPortalNavigationPolicy created for volume " - << m_volume->volumeName()); -} - -void TryAllPortalNavigationPolicy::updateState( - const NavigationArguments& args) const { - const Logger& logger = args.logger; - ACTS_VERBOSE("TryAllPortalNavigationPolicy"); - assert(m_volume != nullptr); - - for (const auto& portal : m_volume->portals()) { - args.main.addPortalCandidate(portal); - }; -} - -void TryAllPortalNavigationPolicy::connect(NavigationDelegate& delegate) const { - connectDefault(delegate); -} - -TryAllSurfaceNavigationPolicy::TryAllSurfaceNavigationPolicy( - const GeometryContext& /*gctx*/, const TrackingVolume& volume, - const Logger& logger) - : m_volume(&volume) { - ACTS_VERBOSE("TryAllSurfaceNavigationPolicy created for volume " - << m_volume->volumeName()); -} - -void TryAllSurfaceNavigationPolicy::updateState( - const NavigationArguments& args) const { - const Logger& logger = args.logger; - ACTS_VERBOSE("TryAllSurfaceNavigationPolicy"); - assert(m_volume != nullptr); - - for (const auto& surface : m_volume->surfaces()) { - args.main.addSurfaceCandidate(surface, args.tolerance); - }; -} - -void TryAllSurfaceNavigationPolicy::connect( - NavigationDelegate& delegate) const { - connectDefault(delegate); -} - -} // namespace Acts diff --git a/Core/src/Navigation/TryAllNavigationPolicy.cpp b/Core/src/Navigation/TryAllNavigationPolicy.cpp new file mode 100644 index 00000000000..df423bfc9b8 --- /dev/null +++ b/Core/src/Navigation/TryAllNavigationPolicy.cpp @@ -0,0 +1,53 @@ +// 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/Navigation/TryAllNavigationPolicy.hpp" + +#include "Acts/Geometry/TrackingVolume.hpp" +#include "Acts/Navigation/NavigationStream.hpp" + +namespace Acts { + +TryAllNavigationPolicy::TryAllNavigationPolicy(const Config& config, + const GeometryContext& /*gctx*/, + const TrackingVolume& volume, + const Logger& logger) + : m_cfg{config}, m_volume(&volume) { + assert(m_volume != nullptr); + ACTS_VERBOSE("TryAllPortalNavigationPolicy created for volume " + << m_volume->volumeName()); +} + +TryAllNavigationPolicy::TryAllNavigationPolicy(const GeometryContext& gctx, + const TrackingVolume& volume, + const Logger& logger) + : TryAllNavigationPolicy({}, gctx, volume, logger) {} + +void TryAllNavigationPolicy::initializeCandidates( + const NavigationArguments& args, AppendOnlyNavigationStream& stream, + const Logger& logger) const { + ACTS_VERBOSE("TryAllPortalNavigationPolicy"); + assert(m_volume != nullptr); + + if (m_cfg.portals) { + for (const auto& portal : m_volume->portals()) { + stream.addPortalCandidate(portal); + } + } + + if (m_cfg.sensitives) + for (const auto& surface : m_volume->surfaces()) { + stream.addSurfaceCandidate(surface, args.tolerance); + }; +} + +void TryAllNavigationPolicy::connect(NavigationDelegate& delegate) const { + connectDefault(delegate); +} + +} // namespace Acts diff --git a/Examples/Python/src/Navigation.cpp b/Examples/Python/src/Navigation.cpp index 57dd62b2ae1..32153cccaf6 100644 --- a/Examples/Python/src/Navigation.cpp +++ b/Examples/Python/src/Navigation.cpp @@ -10,7 +10,7 @@ #include "Acts/Geometry/NavigationPolicyFactory.hpp" #include "Acts/Geometry/TrackingVolume.hpp" #include "Acts/Navigation/SurfaceArrayNavigationPolicy.hpp" -#include "Acts/Navigation/TryAllNavigationPolicies.hpp" +#include "Acts/Navigation/TryAllNavigationPolicy.hpp" #include "Acts/Plugins/Python/Utilities.hpp" #include "Acts/Utilities/Logger.hpp" #include "Acts/Utilities/TypeTag.hpp" @@ -30,10 +30,7 @@ namespace Acts::Python { struct AnyNavigationPolicyFactory : public Acts::NavigationPolicyFactory { virtual std::unique_ptr add( - TypeTag /*type*/) = 0; - - virtual std::unique_ptr add( - TypeTag /*type*/) = 0; + TypeTag /*type*/) = 0; virtual std::unique_ptr add( TypeTag /*type*/, @@ -52,13 +49,8 @@ struct NavigationPolicyFactoryT : public AnyNavigationPolicyFactory { : m_impl{} {} std::unique_ptr add( - TypeTag /*type*/) override { - return add(); - } - - std::unique_ptr add( - TypeTag /*type*/) override { - return add(); + TypeTag /*type*/) override { + return add(); } std::unique_ptr add( @@ -101,11 +93,9 @@ class NavigationPolicyFactory : public Acts::NavigationPolicyFactory { NavigationPolicyFactory& addNoArguments(const py::object& cls) { auto m = py::module_::import("acts"); if (py::object o = m.attr("TryAllPortalNavigationPolicy"); cls.is(o)) { - m_impl = m_impl->add(Type); - } else if (o = m.attr("TryAllSurfaceNavigationPolicy"); cls.is(o)) { - m_impl = m_impl->add(Type); - } else { + m_impl = m_impl->add(Type); } + // Add other policies here return *this; } @@ -134,8 +124,7 @@ void addNavigation(Context& ctx) { std::shared_ptr>( m, "_NavigationPolicyFactory"); - py::class_(m, "TryAllPortalNavigationPolicy"); - py::class_(m, "TryAllSurfaceNavigationPolicy"); + py::class_(m, "TryAllNavigationPolicy"); py::class_>( diff --git a/Examples/Python/tests/test_navigation.py b/Examples/Python/tests/test_navigation.py index 9178cad6854..8723049b061 100644 --- a/Examples/Python/tests/test_navigation.py +++ b/Examples/Python/tests/test_navigation.py @@ -9,8 +9,7 @@ def test_navigation_policy_factory(): policy = ( acts.NavigationPolicyFactory.make() - .add(acts.TryAllPortalNavigationPolicy) - .add(acts.TryAllSurfaceNavigationPolicy) + .add(acts.TryAllNavigationPolicy) .add( acts.SurfaceArrayNavigationPolicy, acts.SurfaceArrayNavigationPolicy.Config( @@ -22,11 +21,7 @@ def test_navigation_policy_factory(): policy._buildTest() - policy = ( - acts.NavigationPolicyFactory.make() - .add(acts.TryAllPortalNavigationPolicy) - .add(acts.TryAllSurfaceNavigationPolicy) - ) + policy = acts.NavigationPolicyFactory.make().add(acts.TryAllNavigationPolicy) policy._buildTest() @@ -42,7 +37,6 @@ def test_navigation_policy_factory_add_multiple(): with pytest.raises(ValueError): ( acts.NavigationPolicyFactory.make() - .add(acts.TryAllPortalNavigationPolicy) - .add(acts.TryAllSurfaceNavigationPolicy) - .add(acts.TryAllPortalNavigationPolicy) + .add(acts.TryAllNavigationPolicy) + .add(acts.TryAllNavigationPolicy) ) diff --git a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp index febec7031f9..e98b5703410 100644 --- a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp +++ b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp @@ -16,7 +16,7 @@ #include "Acts/Navigation/MultiNavigationPolicy.hpp" #include "Acts/Navigation/NavigationDelegate.hpp" #include "Acts/Navigation/NavigationStream.hpp" -#include "Acts/Navigation/TryAllNavigationPolicies.hpp" +#include "Acts/Navigation/TryAllNavigationPolicy.hpp" using namespace Acts; using namespace Acts::UnitLiterals; @@ -30,12 +30,14 @@ struct APolicy : public INavigationPolicy { APolicy(const GeometryContext& /*gctx*/, const TrackingVolume& /*volume*/, const Logger& /*logger*/) {} - void updateState(const NavigationArguments& /*unused*/) const { + void initializeCandidates(const NavigationArguments& /*unused*/, + AppendOnlyNavigationStream& /*unused*/, + const Logger& /*unused*/) const { const_cast(this)->executed = true; } void connect(NavigationDelegate& delegate) const override { - connectDefault(delegate); + connectDefault(delegate); } bool executed = false; @@ -51,10 +53,12 @@ struct BPolicy : public INavigationPolicy { : m_config(config) {} void connect(NavigationDelegate& delegate) const override { - connectDefault(delegate); + connectDefault(delegate); } - void updateState(const NavigationArguments& /*unused*/) const { + void initializeCandidates(const NavigationArguments& /*unused*/, + AppendOnlyNavigationStream& /*unused*/, + const Logger& /*unused*/) const { const_cast(this)->executed = true; const_cast(this)->value = m_config.value; } @@ -78,10 +82,10 @@ BOOST_AUTO_TEST_CASE(DirectTest) { policy.connect(delegate); NavigationStream main; - delegate(NavigationArguments{.main = main, - .position = Vector3::Zero(), - .direction = Vector3::Zero(), - .logger = *logger}); + AppendOnlyNavigationStream stream{main}; + delegate(NavigationArguments{.position = Vector3::Zero(), + .direction = Vector3::Zero()}, + stream, *logger); BOOST_CHECK(std::get(policy.policies()).executed); BOOST_CHECK(std::get(policy.policies()).executed); @@ -112,10 +116,10 @@ BOOST_AUTO_TEST_CASE(FactoryTest) { policy.connect(delegate); NavigationStream main; - delegate(NavigationArguments{.main = main, - .position = Vector3::Zero(), - .direction = Vector3::Zero(), - .logger = *logger}); + AppendOnlyNavigationStream stream{main}; + delegate(NavigationArguments{.position = Vector3::Zero(), + .direction = Vector3::Zero()}, + stream, *logger); BOOST_CHECK(std::get(policy.policies()).executed); BOOST_CHECK(std::get(policy.policies()).executed); @@ -127,10 +131,9 @@ BOOST_AUTO_TEST_CASE(FactoryTest) { NavigationDelegate delegate2; policyBase2->connect(delegate2); - delegate2(NavigationArguments{.main = main, - .position = Vector3::Zero(), - .direction = Vector3::Zero(), - .logger = *logger}); + delegate(NavigationArguments{.position = Vector3::Zero(), + .direction = Vector3::Zero()}, + stream, *logger); BOOST_CHECK(std::get(policy2.policies()).executed); BOOST_CHECK(std::get(policy2.policies()).executed); @@ -153,10 +156,10 @@ BOOST_AUTO_TEST_CASE(AsUniquePtrTest) { policyBase->connect(delegate); NavigationStream main; - delegate(NavigationArguments{.main = main, - .position = Vector3::Zero(), - .direction = Vector3::Zero(), - .logger = *logger}); + AppendOnlyNavigationStream stream{main}; + delegate(NavigationArguments{.position = Vector3::Zero(), + .direction = Vector3::Zero()}, + stream, *logger); BOOST_CHECK(std::get(policy.policies()).executed); } @@ -173,10 +176,12 @@ struct CPolicySpecialized : public CPolicy { : m_config(config) {} void connect(NavigationDelegate& delegate) const override { - connectDefault(delegate); + connectDefault>(delegate); } - void updateState(const NavigationArguments& /*unused*/) const { + void initializeCandidates(const NavigationArguments& /*unused*/, + AppendOnlyNavigationStream& /*stream*/, + const Logger& /*logger*/) const { auto* self = const_cast*>(this); self->executed = true; self->value = m_config.value; @@ -221,10 +226,10 @@ BOOST_AUTO_TEST_CASE(IsolatedFactory) { policyBase->connect(delegate); NavigationStream main; - delegate(NavigationArguments{.main = main, - .position = Vector3::Zero(), - .direction = Vector3::Zero(), - .logger = *logger}); + AppendOnlyNavigationStream stream{main}; + delegate(NavigationArguments{.position = Vector3::Zero(), + .direction = Vector3::Zero()}, + stream, *logger); BOOST_CHECK(std::get(policy.policies()).executed); BOOST_CHECK(std::get>(policy.policies()).executed); From e87497f9f4198b51e86d18978692fc03f001b48f Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 23 Oct 2024 12:17:52 +0200 Subject: [PATCH 20/25] fix unit test --- Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp index e98b5703410..181e1ad2a1f 100644 --- a/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp +++ b/Tests/UnitTests/Core/Navigation/NavigationPolicyTests.cpp @@ -131,9 +131,9 @@ BOOST_AUTO_TEST_CASE(FactoryTest) { NavigationDelegate delegate2; policyBase2->connect(delegate2); - delegate(NavigationArguments{.position = Vector3::Zero(), - .direction = Vector3::Zero()}, - stream, *logger); + delegate2(NavigationArguments{.position = Vector3::Zero(), + .direction = Vector3::Zero()}, + stream, *logger); BOOST_CHECK(std::get(policy2.policies()).executed); BOOST_CHECK(std::get(policy2.policies()).executed); From c859c950113c5e0a69011906322ea07fcc19effe Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 23 Oct 2024 12:22:57 +0200 Subject: [PATCH 21/25] fix python bindings issues + logger --- Core/src/Navigation/TryAllNavigationPolicy.cpp | 4 ++-- Examples/Python/src/Navigation.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Core/src/Navigation/TryAllNavigationPolicy.cpp b/Core/src/Navigation/TryAllNavigationPolicy.cpp index df423bfc9b8..23b0a818282 100644 --- a/Core/src/Navigation/TryAllNavigationPolicy.cpp +++ b/Core/src/Navigation/TryAllNavigationPolicy.cpp @@ -19,7 +19,7 @@ TryAllNavigationPolicy::TryAllNavigationPolicy(const Config& config, const Logger& logger) : m_cfg{config}, m_volume(&volume) { assert(m_volume != nullptr); - ACTS_VERBOSE("TryAllPortalNavigationPolicy created for volume " + ACTS_VERBOSE("TryAllNavigationPolicy created for volume " << m_volume->volumeName()); } @@ -31,7 +31,7 @@ TryAllNavigationPolicy::TryAllNavigationPolicy(const GeometryContext& gctx, void TryAllNavigationPolicy::initializeCandidates( const NavigationArguments& args, AppendOnlyNavigationStream& stream, const Logger& logger) const { - ACTS_VERBOSE("TryAllPortalNavigationPolicy"); + ACTS_VERBOSE("TryAllNavigationPolicy"); assert(m_volume != nullptr); if (m_cfg.portals) { diff --git a/Examples/Python/src/Navigation.cpp b/Examples/Python/src/Navigation.cpp index 32153cccaf6..c7e3df8779e 100644 --- a/Examples/Python/src/Navigation.cpp +++ b/Examples/Python/src/Navigation.cpp @@ -92,7 +92,7 @@ class NavigationPolicyFactory : public Acts::NavigationPolicyFactory { // arguments NavigationPolicyFactory& addNoArguments(const py::object& cls) { auto m = py::module_::import("acts"); - if (py::object o = m.attr("TryAllPortalNavigationPolicy"); cls.is(o)) { + if (py::object o = m.attr("TryAllNavigationPolicy"); cls.is(o)) { m_impl = m_impl->add(Type); } // Add other policies here From fdf63f9db812c4a7e6b1564421c159fce7a8eb21 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 23 Oct 2024 16:27:02 +0200 Subject: [PATCH 22/25] fix python test --- Examples/Python/src/Navigation.cpp | 37 ++++++++++++++++++++++++ Examples/Python/tests/test_navigation.py | 2 +- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/Examples/Python/src/Navigation.cpp b/Examples/Python/src/Navigation.cpp index c7e3df8779e..ac2f1a77ef6 100644 --- a/Examples/Python/src/Navigation.cpp +++ b/Examples/Python/src/Navigation.cpp @@ -12,6 +12,8 @@ #include "Acts/Navigation/SurfaceArrayNavigationPolicy.hpp" #include "Acts/Navigation/TryAllNavigationPolicy.hpp" #include "Acts/Plugins/Python/Utilities.hpp" +#include "Acts/Surfaces/CylinderBounds.hpp" +#include "Acts/Surfaces/CylinderSurface.hpp" #include "Acts/Utilities/Logger.hpp" #include "Acts/Utilities/TypeTag.hpp" @@ -117,6 +119,33 @@ class NavigationPolicyFactory : public Acts::NavigationPolicyFactory { std::make_unique>(); }; +namespace Test { +class DetectorElementStub : public DetectorElementBase { + public: + DetectorElementStub() : DetectorElementBase() {} + + const Transform3& transform(const GeometryContext&) const override { + return m_transform; + } + + /// Return surface representation - const return pattern + const Surface& surface() const override { + throw std::runtime_error("Not implemented"); + } + + /// Non-const return pattern + Surface& surface() override { throw std::runtime_error("Not implemented"); } + + /// Returns the thickness of the module + /// @return double that indicates the thickness of the module + double thickness() const override { return 0; } + + private: + Transform3 m_transform; +}; + +} // namespace Test + void addNavigation(Context& ctx) { auto m = ctx.get("main"); @@ -139,6 +168,14 @@ void addNavigation(Context& ctx) { std::make_shared(30, 40, 100)); vol1->setVolumeName("TestVolume"); + auto detElem = std::make_unique(); + + auto surface = Surface::makeShared( + Transform3::Identity(), std::make_shared(30, 40)); + surface->assignDetectorElement(*detElem); + + vol1->addSurface(std::move(surface)); + std::unique_ptr result = self.build(GeometryContext{}, *vol1, *getDefaultLogger("Test", Logging::VERBOSE)); diff --git a/Examples/Python/tests/test_navigation.py b/Examples/Python/tests/test_navigation.py index 8723049b061..55ec8ac9540 100644 --- a/Examples/Python/tests/test_navigation.py +++ b/Examples/Python/tests/test_navigation.py @@ -13,7 +13,7 @@ def test_navigation_policy_factory(): .add( acts.SurfaceArrayNavigationPolicy, acts.SurfaceArrayNavigationPolicy.Config( - layerType=acts.SurfaceArrayNavigationPolicy.LayerType.Plane, + layerType=acts.SurfaceArrayNavigationPolicy.LayerType.Disc, bins=(10, 10), ), ) From 5946ab8ae30a58f57bc68e99d6126232cb776bc0 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 23 Oct 2024 16:28:41 +0200 Subject: [PATCH 23/25] fix clang-tidy + docs --- Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp | 4 ++++ Core/src/Navigation/TryAllNavigationPolicy.cpp | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp b/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp index bef9dab6e0d..c013cb03417 100644 --- a/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp @@ -34,6 +34,10 @@ class TryAllNavigationPolicy final : public INavigationPolicy { TryAllNavigationPolicy(const Config& config, const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger); + /// Constructor from a volume + /// @param gctx is the geometry context + /// @param volume is the volume to navigate + /// @param logger is the logger TryAllNavigationPolicy(const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger); diff --git a/Core/src/Navigation/TryAllNavigationPolicy.cpp b/Core/src/Navigation/TryAllNavigationPolicy.cpp index 23b0a818282..40ca70c4079 100644 --- a/Core/src/Navigation/TryAllNavigationPolicy.cpp +++ b/Core/src/Navigation/TryAllNavigationPolicy.cpp @@ -40,10 +40,11 @@ void TryAllNavigationPolicy::initializeCandidates( } } - if (m_cfg.sensitives) + if (m_cfg.sensitives) { for (const auto& surface : m_volume->surfaces()) { stream.addSurfaceCandidate(surface, args.tolerance); }; + } } void TryAllNavigationPolicy::connect(NavigationDelegate& delegate) const { From b8d52c463eac1223c26f908093c2741b23ffb4a6 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 23 Oct 2024 18:27:53 +0200 Subject: [PATCH 24/25] another doc comment fix --- Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp b/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp index c013cb03417..e7dc4bb3ddc 100644 --- a/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/TryAllNavigationPolicy.hpp @@ -41,8 +41,10 @@ class TryAllNavigationPolicy final : public INavigationPolicy { TryAllNavigationPolicy(const GeometryContext& gctx, const TrackingVolume& volume, const Logger& logger); - /// Update the navigation state with all volume portals. + /// Add all candidates to the stream /// @param args are the navigation arguments + /// @param stream is the navigation stream to update + /// @param logger is the logger void initializeCandidates(const NavigationArguments& args, AppendOnlyNavigationStream& stream, const Logger& logger) const; From 16602adeedef8ad2a07a6e93575b943ccc4c33ab Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 24 Oct 2024 11:23:07 +0200 Subject: [PATCH 25/25] sonar fix --- Core/include/Acts/Navigation/MultiNavigationPolicy.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp b/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp index 67c1af6bde4..c5c8a397aea 100644 --- a/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp +++ b/Core/include/Acts/Navigation/MultiNavigationPolicy.hpp @@ -31,7 +31,7 @@ class MultiNavigationPolicy final : public MultiNavigationPolicyBase { /// Constructor from a set of child policies. /// @param policies The child policies MultiNavigationPolicy(Policies&&... policies) - : m_policies{std::forward(policies)...} {} + : m_policies{std::move(policies)...} {} /// Implementation of the connection to a navigation delegate. /// It uses the delegate chain factory to produce a delegate chain and stores