Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Decouple SurfaceAccessor from source link implementations #3445

Merged
merged 18 commits into from
Aug 6, 2024
7 changes: 7 additions & 0 deletions Core/include/Acts/Detector/Detector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,13 @@ class Detector : public std::enable_shared_from_this<Detector> {
/// @return the map which can be queried with GeometryID for ranges
const GeometryHierarchyMap<const Surface*>& sensitiveHierarchyMap() const;

/// Search for a surface with the given identifier.
///
/// @param id is the geometry identifier of the surface
/// @retval nullptr if no such surface exists
/// @retval pointer to the found surface otherwise.
const Surface* findSurface(GeometryIdentifier id) const;

/// @brief Visit all reachable surfaces of the detector
///
/// @tparam visitor_t Type of the callable visitor
Expand Down
32 changes: 25 additions & 7 deletions Core/include/Acts/EventData/detail/TestSourceLink.hpp
ssdetlab marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "Acts/Definitions/Algebra.hpp"
#include "Acts/Definitions/TrackParametrization.hpp"
#include "Acts/Detector/Detector.hpp"
#include "Acts/EventData/MultiTrajectory.hpp"
#include "Acts/EventData/SourceLink.hpp"
#include "Acts/Geometry/GeometryContext.hpp"
Expand All @@ -27,6 +28,8 @@

namespace Acts::detail::Test {

struct TestSourceLinkSurfaceAccessor;

/// A minimal source link implementation for testing.
///
/// Instead of storing a reference to a measurement or raw data, the measurement
Expand All @@ -35,6 +38,8 @@ namespace Acts::detail::Test {
/// identifier is stored that can be used to store additional information. How
/// this is interpreted depends on the specific tests.
struct TestSourceLink final {
using SurfaceAccessor = TestSourceLinkSurfaceAccessor;

GeometryIdentifier m_geometryId{};
std::size_t sourceId = 0u;
// use eBoundSize to indicate unused indices
Expand Down Expand Up @@ -87,17 +92,30 @@ struct TestSourceLink final {
return os;
}
constexpr std::size_t index() const { return sourceId; }
};

struct SurfaceAccessor {
const Acts::TrackingGeometry& trackingGeometry;
andiwand marked this conversation as resolved.
Show resolved Hide resolved
struct TestSourceLinkSurfaceAccessor {
const TrackingGeometry& geometry;

const Acts::Surface* operator()(const Acts::SourceLink& sourceLink) const {
const auto& testSourceLink = sourceLink.get<TestSourceLink>();
return trackingGeometry.findSurface(testSourceLink.m_geometryId);
}
};
const Acts::Surface* operator()(const Acts::SourceLink& sourceLink) const {
const auto& testSourceLink = sourceLink.get<TestSourceLink>();
return geometry.findSurface(testSourceLink.m_geometryId);
}
};

namespace Experimental {

struct TestSourceLinkSurfaceAccessor {
const Acts::Experimental::Detector& geometry;

const Acts::Surface* operator()(const Acts::SourceLink& sourceLink) const {
const auto& testSourceLink = sourceLink.get<TestSourceLink>();
return geometry.findSurface(testSourceLink.m_geometryId);
}
};

} // namespace Experimental

inline std::ostream& operator<<(std::ostream& os,
const TestSourceLink& sourceLink) {
return sourceLink.print(os);
Expand Down
5 changes: 5 additions & 0 deletions Core/src/Detector/Detector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,8 @@ const Acts::GeometryHierarchyMap<const Acts::Surface*>&
Acts::Experimental::Detector::sensitiveHierarchyMap() const {
return m_sensitiveHierarchyMap;
}

const Acts::Surface* Acts::Experimental::Detector::findSurface(
GeometryIdentifier geoID) const {
return *m_sensitiveHierarchyMap.find(geoID);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#pragma once

#include "Acts/Detector/Detector.hpp"
#include "Acts/EventData/SourceLink.hpp"
#include "Acts/Geometry/TrackingGeometry.hpp"
#include "Acts/Surfaces/Surface.hpp"
Expand All @@ -18,6 +19,8 @@

namespace ActsExamples {

struct IndexSourceLinkSurfaceAccessor;

/// A source link that stores just an index.
///
/// This is intentionally kept as barebones as possible. The source link
Expand All @@ -29,6 +32,8 @@ namespace ActsExamples {
/// easily changed without having to also change the source link.
class IndexSourceLink final {
public:
using SurfaceAccessor = IndexSourceLinkSurfaceAccessor;

/// Construct from geometry identifier and index.
constexpr IndexSourceLink(Acts::GeometryIdentifier gid, Index idx)
: m_geometryId(gid), m_index(idx) {}
Expand All @@ -46,15 +51,6 @@ class IndexSourceLink final {

Acts::GeometryIdentifier geometryId() const { return m_geometryId; }

struct SurfaceAccessor {
const Acts::TrackingGeometry& trackingGeometry;

const Acts::Surface* operator()(const Acts::SourceLink& sourceLink) const {
const auto& indexSourceLink = sourceLink.get<IndexSourceLink>();
return trackingGeometry.findSurface(indexSourceLink.geometryId());
}
};

private:
Acts::GeometryIdentifier m_geometryId;
Index m_index = 0;
Expand All @@ -70,6 +66,28 @@ class IndexSourceLink final {
}
};

struct IndexSourceLinkSurfaceAccessor {
const Acts::TrackingGeometry& geometry;

const Acts::Surface* operator()(const Acts::SourceLink& sourceLink) const {
const auto& indexSourceLink = sourceLink.get<IndexSourceLink>();
return geometry.findSurface(indexSourceLink.geometryId());
}
};

namespace Experimental {

struct IndexSourceLinkSurfaceAccessor {
const Acts::Experimental::Detector& geometry;

const Acts::Surface* operator()(const Acts::SourceLink& sourceLink) const {
const auto& indexSourceLink = sourceLink.get<IndexSourceLink>();
return geometry.findSurface(indexSourceLink.geometryId());
}
};

} // namespace Experimental

/// Container of index source links.
///
/// Since the source links provide a `.geometryId()` accessor, they can be
Expand Down
1 change: 1 addition & 0 deletions Tests/UnitTests/Core/EventData/SourceLinkTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ BOOST_AUTO_TEST_SUITE(EventDataSourceLink)

BOOST_AUTO_TEST_CASE(TestSourceLinkCoverage) {
using Acts::detail::Test::TestSourceLink;

TestSourceLink ts;
Acts::Vector2 stddev(0.01, 0.1);
Acts::SquareMatrix2 cov = stddev.cwiseProduct(stddev).asDiagonal();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ namespace Acts::Test {

using StraightPropagator = Propagator<StraightLineStepper, Navigator>;
using TestSourceLink = detail::Test::TestSourceLink;
using SurfaceAccessor = detail::Test::TestSourceLink::SurfaceAccessor;
using ConstantFieldStepper = EigenStepper<>;
using ConstantFieldPropagator = Propagator<ConstantFieldStepper, Navigator>;
// Construct initial track parameters.
Expand Down Expand Up @@ -186,18 +187,18 @@ BOOST_DATA_TEST_CASE(SpacePointBuilder_basic, bdata::xrange(1), index) {
auto spBuilderConfig = SpacePointBuilderConfig();
spBuilderConfig.trackingGeometry = geometry;

TestSourceLink::SurfaceAccessor surfaceAccessor{*geometry};
spBuilderConfig.slSurfaceAccessor
.connect<&TestSourceLink::SurfaceAccessor::operator()>(&surfaceAccessor);
SurfaceAccessor surfaceAccessor{*geometry};
spBuilderConfig.slSurfaceAccessor.connect<&SurfaceAccessor::operator()>(
&surfaceAccessor);

auto spBuilder =
SpacePointBuilder<TestSpacePoint>(spBuilderConfig, spConstructor);

// for cosmic without vertex constraint, usePerpProj = true
auto spBuilderConfig_perp = SpacePointBuilderConfig();
spBuilderConfig_perp.trackingGeometry = geometry;
spBuilderConfig_perp.slSurfaceAccessor
.connect<&TestSourceLink::SurfaceAccessor::operator()>(&surfaceAccessor);
spBuilderConfig_perp.slSurfaceAccessor.connect<&SurfaceAccessor::operator()>(
&surfaceAccessor);

spBuilderConfig_perp.usePerpProj = true;

Expand Down Expand Up @@ -280,8 +281,7 @@ BOOST_DATA_TEST_CASE(SpacePointBuilder_basic, bdata::xrange(1), index) {

spBuilderConfig_badStrips.trackingGeometry = geometry;
spBuilderConfig_badStrips.slSurfaceAccessor
.connect<&TestSourceLink::SurfaceAccessor::operator()>(
&surfaceAccessor);
.connect<&SurfaceAccessor::operator()>(&surfaceAccessor);

auto spBuilder_badStrips = SpacePointBuilder<TestSpacePoint>(
spBuilderConfig_badStrips, spConstructor);
Expand Down
Loading