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

refactor: resurrect detector root volumes #2044

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions Core/include/Acts/Detector/Detector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,21 @@ class Detector : public std::enable_shared_from_this<Detector> {
/// Create a detector from volumes
///
/// @param name the detecor name
/// @param rootVolume the root detector volume
/// @param rootVolumes the volumes contained by this detector
/// @param detectorVolumeUpdator is a Delegate to find the assocaited volume
///
/// @note will throw an exception if volumes vector is empty
/// @note will throw an exception if duplicate volume names exist
/// @note will throw an exception if the delegate is not connected
Detector(const std::string& name, std::shared_ptr<DetectorVolume> rootVolume,
Detector(const std::string& name,
std::vector<std::shared_ptr<DetectorVolume>> rootVolumes,
DetectorVolumeUpdator&& detectorVolumeUpdator) noexcept(false);

public:
/// Factory for producing memory managed instances of Detector.
static std::shared_ptr<Detector> makeShared(
const std::string& name, std::shared_ptr<DetectorVolume> rootVolume,
const std::string& name,
std::vector<std::shared_ptr<DetectorVolume>> rootVolumes,
DetectorVolumeUpdator&& detectorVolumeUpdator);

/// Retrieve a @c std::shared_ptr for this surface (non-const version)
Expand All @@ -66,15 +68,15 @@ class Detector : public std::enable_shared_from_this<Detector> {
/// @return The shared pointer
std::shared_ptr<const Detector> getSharedPtr() const;

/// Non-const access to the root volume
/// Non-const access to the root volumes
///
/// @return the root volume shared pointer
const std::shared_ptr<DetectorVolume>& rootVolumePtr();
std::vector<std::shared_ptr<DetectorVolume>>& rootVolumePtrs();

/// Const access to the root volume
/// Const access to the root volumes
///
/// @return a vector to const DetectorVolume raw pointers
const DetectorVolume* rootVolume() const;
const std::vector<const DetectorVolume*>& rootVolumes() const;

/// Non-const access to the root volume
///
Expand Down Expand Up @@ -128,8 +130,8 @@ class Detector : public std::enable_shared_from_this<Detector> {
/// Name of the detector
std::string m_name = "Unnamed";

/// Root volume
std::shared_ptr<DetectorVolume> m_rootVolume;
/// Root volumes
DetectorVolume::ObjectStore<std::shared_ptr<DetectorVolume>> m_rootVolumes;

/// Volume store (internal/external)
DetectorVolume::ObjectStore<std::shared_ptr<DetectorVolume>> m_volumes;
Expand Down
8 changes: 4 additions & 4 deletions Core/include/Acts/Detector/DetectorVolume.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
DetectorVolume(
const GeometryContext& gctx, const std::string& name,
const Transform3& transform, std::unique_ptr<VolumeBounds> bounds,
const std::vector<std::shared_ptr<Surface>>& surfaces,
const std::vector<std::shared_ptr<DetectorVolume>>& volumes,
std::vector<std::shared_ptr<Surface>> surfaces,
std::vector<std::shared_ptr<DetectorVolume>> volumes,
DetectorVolumeUpdator&& detectorVolumeUpdator,
SurfaceCandidatesUpdator&& surfaceCandidateUpdator) noexcept(false);

Expand All @@ -129,8 +129,8 @@ class DetectorVolume : public std::enable_shared_from_this<DetectorVolume> {
static std::shared_ptr<DetectorVolume> makeShared(
const GeometryContext& gctx, const std::string& name,
const Transform3& transform, std::unique_ptr<VolumeBounds> bounds,
const std::vector<std::shared_ptr<Surface>>& surfaces,
const std::vector<std::shared_ptr<DetectorVolume>>& volumes,
std::vector<std::shared_ptr<Surface>> surfaces,
std::vector<std::shared_ptr<DetectorVolume>> volumes,
DetectorVolumeUpdator&& detectorVolumeUpdator,
SurfaceCandidatesUpdator&& surfaceCandidateUpdator);

Expand Down
16 changes: 9 additions & 7 deletions Core/include/Acts/Navigation/DetectorVolumeFinders.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ struct RootVolumeFinder : public INavigationDelegate {
"DetectorVolumeFinders: no detector set to navigation state.");
}

const auto& volume = nState.currentDetector->rootVolume();
if (volume->inside(gctx, nState.position)) {
nState.currentVolume = volume;
volume->detectorVolumeUpdator()(gctx, nState);
return;
const auto& volumes = nState.currentDetector->rootVolumes();
for (const auto v : volumes) {
if (v->inside(gctx, nState.position)) {
nState.currentVolume = v;
v->detectorVolumeUpdator()(gctx, nState);
return;
}
}
nState.currentVolume = nullptr;
}
Expand All @@ -65,8 +67,8 @@ struct TrialAndErrorVolumeFinder : public INavigationDelegate {
}
};

/// Generate a delegate to try the root volume
inline static DetectorVolumeUpdator tryRootVolume() {
/// Generate a delegate to try the root volumes
inline static DetectorVolumeUpdator tryRootVolumes() {
DetectorVolumeUpdator vFinder;
vFinder.connect<&RootVolumeFinder::update>(
std::make_unique<const RootVolumeFinder>());
Expand Down
41 changes: 23 additions & 18 deletions Core/src/Detector/Detector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@
#include "Acts/Utilities/Helpers.hpp"

Acts::Experimental::Detector::Detector(
const std::string& name, std::shared_ptr<DetectorVolume> rootVolume,
const std::string& name,
std::vector<std::shared_ptr<DetectorVolume>> rootVolumes,
DetectorVolumeUpdator&& detectorVolumeUpdator)
: m_name(name),
m_rootVolume(std::move(rootVolume)),
m_rootVolumes(std::move(rootVolumes)),
m_detectorVolumeUpdator(std::move(detectorVolumeUpdator)) {
if (not m_rootVolume) {
if (m_rootVolumes.internal.empty()) {
throw std::invalid_argument("Detector: no volume were given.");
}
if (not m_detectorVolumeUpdator.connected()) {
Expand All @@ -31,19 +32,22 @@ Acts::Experimental::Detector::Detector(
}

// Fill volumes
auto collectVolumes = [](DetectorVolume& root) {
auto collectVolumes = [&]() {
std::vector<std::shared_ptr<DetectorVolume>> volumes;
auto recurse = [&volumes](DetectorVolume& volume, auto& callback) -> void {
for (const auto& v : volume.volumePtrs()) {
volumes.push_back(v);
callback(*v, callback);
auto recurse = [&volumes](const std::shared_ptr<DetectorVolume>& volume,
auto& callback) -> void {
volumes.push_back(volume);
for (const auto& v : volume->volumePtrs()) {
callback(v, callback);
}
};
recurse(root, recurse);
for (const auto& root : m_rootVolumes.internal) {
recurse(root, recurse);
}
return volumes;
};
m_volumes = DetectorVolume::ObjectStore<std::shared_ptr<DetectorVolume>>(
collectVolumes(*m_rootVolume));
collectVolumes());

// Check for unique names and fill the volume name / index map
for (auto [iv, v] : enumerate(m_volumes.internal)) {
Expand All @@ -63,20 +67,21 @@ Acts::Experimental::Detector::Detector(

std::shared_ptr<Acts::Experimental::Detector>
Acts::Experimental::Detector::makeShared(
const std::string& name, std::shared_ptr<DetectorVolume> rootVolume,
const std::string& name,
std::vector<std::shared_ptr<DetectorVolume>> rootVolumes,
DetectorVolumeUpdator&& detectorVolumeUpdator) {
return std::shared_ptr<Detector>(new Detector(
name, std::move(rootVolume), std::move(detectorVolumeUpdator)));
name, std::move(rootVolumes), std::move(detectorVolumeUpdator)));
}

const std::shared_ptr<Acts::Experimental::DetectorVolume>&
Acts::Experimental::Detector::rootVolumePtr() {
return m_rootVolume;
std::vector<std::shared_ptr<Acts::Experimental::DetectorVolume>>&
Acts::Experimental::Detector::rootVolumePtrs() {
return m_rootVolumes.internal;
}

const Acts::Experimental::DetectorVolume*
Acts::Experimental::Detector::rootVolume() const {
return m_rootVolume.get();
const std::vector<const Acts::Experimental::DetectorVolume*>&
Acts::Experimental::Detector::rootVolumes() const {
return m_rootVolumes.external;
}

std::vector<std::shared_ptr<Acts::Experimental::DetectorVolume>>&
Expand Down
18 changes: 9 additions & 9 deletions Core/src/Detector/DetectorVolume.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@
Acts::Experimental::DetectorVolume::DetectorVolume(
const GeometryContext& gctx, const std::string& name,
const Transform3& transform, std::unique_ptr<VolumeBounds> bounds,
const std::vector<std::shared_ptr<Surface>>& surfaces,
const std::vector<std::shared_ptr<DetectorVolume>>& volumes,
std::vector<std::shared_ptr<Surface>> surfaces,
std::vector<std::shared_ptr<DetectorVolume>> volumes,
DetectorVolumeUpdator&& detectorVolumeUpdator,
SurfaceCandidatesUpdator&& surfaceCandidateUpdator)
: m_name(name),
m_transform(transform),
m_bounds(std::move(bounds)),
m_surfaces(std::move(surfaces)),
m_volumes(std::move(volumes)),
m_detectorVolumeUpdator(std::move(detectorVolumeUpdator)),
m_surfaceCandidatesUpdator(std::move(surfaceCandidateUpdator)),
m_volumeMaterial(nullptr) {
Expand All @@ -45,9 +47,6 @@ Acts::Experimental::DetectorVolume::DetectorVolume(
"DetectorVolume: navigation state updator delegate is not connected.");
}

m_surfaces = ObjectStore<std::shared_ptr<Surface>>(surfaces);
m_volumes = ObjectStore<std::shared_ptr<DetectorVolume>>(volumes);

[[maybe_unused]] const auto& gctx_ref = gctx;
assert(checkContainment(gctx) && "Objects are not contained by volume.");
}
Expand All @@ -63,13 +62,14 @@ std::shared_ptr<Acts::Experimental::DetectorVolume>
Acts::Experimental::DetectorVolume::makeShared(
const GeometryContext& gctx, const std::string& name,
const Transform3& transform, std::unique_ptr<VolumeBounds> bounds,
const std::vector<std::shared_ptr<Surface>>& surfaces,
const std::vector<std::shared_ptr<DetectorVolume>>& volumes,
std::vector<std::shared_ptr<Surface>> surfaces,
std::vector<std::shared_ptr<DetectorVolume>> volumes,
DetectorVolumeUpdator&& detectorVolumeUpdator,
SurfaceCandidatesUpdator&& surfaceCandidateUpdator) {
return std::shared_ptr<DetectorVolume>(new DetectorVolume(
gctx, name, transform, std::move(bounds), surfaces, volumes,
std::move(detectorVolumeUpdator), std::move(surfaceCandidateUpdator)));
gctx, name, transform, std::move(bounds), std::move(surfaces),
std::move(volumes), std::move(detectorVolumeUpdator),
std::move(surfaceCandidateUpdator)));
}

std::shared_ptr<Acts::Experimental::DetectorVolume>
Expand Down
3 changes: 2 additions & 1 deletion Examples/Run/Geant4/TestMockupBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,6 @@ int main() {
mockup_builder.drawSector(detectorVolume_sector, "sector_test.obj");

auto detector_sector = Acts::Experimental::Detector::makeShared(
"Detector", detectorVolume_sector, Acts::Experimental::tryRootVolume());
"Detector", {detectorVolume_sector},
Acts::Experimental::tryRootVolumes());
}
29 changes: 7 additions & 22 deletions Tests/UnitTests/Core/Detector/DetectorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,6 @@ BOOST_AUTO_TEST_CASE(DetectorConstruction) {
auto cyl2Bounds =
std::make_unique<Acts::CylinderVolumeBounds>(r2, r3, zHalfL);

auto rootBounds =
std::make_unique<Acts::CylinderVolumeBounds>(r0, r3, zHalfL);

auto rootBoundsCopy =
std::make_unique<Acts::CylinderVolumeBounds>(r0, r3, zHalfL);

auto portalGenerator = Acts::Experimental::defaultPortalGenerator();

auto cyl0 = Acts::Experimental::DetectorVolumeFactory::construct(
Expand All @@ -86,13 +80,8 @@ BOOST_AUTO_TEST_CASE(DetectorConstruction) {

std::vector<std::shared_ptr<Acts::Experimental::DetectorVolume>> volumes012 =
{cyl0, cyl1, cyl2};
auto root012 = Acts::Experimental::DetectorVolumeFactory::construct(
portalGenerator, tContext, "root", nominal, std::move(rootBounds),
std::vector<std::shared_ptr<Acts::Surface>>(), volumes012,
Acts::Experimental::tryAllSubVolumes(),
Acts::Experimental::tryAllPortals());
auto det012 = Acts::Experimental::Detector::makeShared(
"Det012", root012, Acts::Experimental::tryRootVolume());
"Det012", volumes012, Acts::Experimental::tryRootVolumes());

// Check the basic return functions
BOOST_CHECK(det012->name() == "Det012");
Expand Down Expand Up @@ -131,21 +120,17 @@ BOOST_AUTO_TEST_CASE(DetectorConstruction) {

// Misconfigured - unkonnected finder
Acts::Experimental::DetectorVolumeUpdator unconnected;
BOOST_CHECK_THROW(Acts::Experimental::Detector::makeShared(
"Det012_unconnected", root012, std::move(unconnected)),
std::invalid_argument);
BOOST_CHECK_THROW(
Acts::Experimental::Detector::makeShared("Det012_unconnected", volumes012,
std::move(unconnected)),
std::invalid_argument);

// Misconfigured - duplicate name
std::vector<std::shared_ptr<Acts::Experimental::DetectorVolume>> volumes002 =
{cyl0, cyl0nameDup, cyl2};
auto root002 = Acts::Experimental::DetectorVolumeFactory::construct(
portalGenerator, tContext, "root", nominal, std::move(rootBoundsCopy),
std::vector<std::shared_ptr<Acts::Surface>>(), volumes002,
Acts::Experimental::tryAllSubVolumes(),
Acts::Experimental::tryAllPortals());
BOOST_CHECK_THROW(Acts::Experimental::Detector::makeShared(
"Det002_name_duplicate", root002,
Acts::Experimental::tryRootVolume()),
"Det002_name_duplicate", volumes002,
Acts::Experimental::tryRootVolumes()),
std::invalid_argument);
}

Expand Down
30 changes: 9 additions & 21 deletions Tests/UnitTests/Core/Navigation/DetectorVolumeFindersTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ auto cyl1Bounds = std::make_unique<Acts::CylinderVolumeBounds>(r1, r2, zHalfL);

auto cyl2Bounds = std::make_unique<Acts::CylinderVolumeBounds>(r2, r3, zHalfL);

auto rootBounds = std::make_unique<Acts::CylinderVolumeBounds>(r0, r3, zHalfL);

auto portalGenerator = Acts::Experimental::defaultPortalGenerator();

auto cyl0 = Acts::Experimental::DetectorVolumeFactory::construct(
Expand All @@ -63,40 +61,30 @@ auto cyl2 = Acts::Experimental::DetectorVolumeFactory::construct(

std::vector<std::shared_ptr<Acts::Experimental::DetectorVolume>> volumes012 = {
cyl0, cyl1, cyl2};

auto root012 = Acts::Experimental::DetectorVolumeFactory::construct(
portalGenerator, tContext, "root", nominal, std::move(rootBounds), {},
volumes012, Acts::Experimental::tryAllSubVolumes(),
Acts::Experimental::tryAllPortals());

auto det012 = Acts::Experimental::Detector::makeShared(
"Det012", root012, Acts::Experimental::tryRootVolume());
"Det012", volumes012, Acts::Experimental::tryRootVolumes());

BOOST_AUTO_TEST_SUITE(Experimental)

// Test finding detectors beu trial and error
BOOST_AUTO_TEST_CASE(TrialAndErrorDetectorVolumeFinder) {
// Test finding detectors by trial and error
BOOST_AUTO_TEST_CASE(RootVolumeFinder) {
nState.currentDetector = det012.get();
nState.currentVolume = root012.get();
Acts::Experimental::TrialAndErrorVolumeFinder tae;
Acts::Experimental::RootVolumeFinder rvf;
// Cylinder 0
nState.position = Acts::Vector3(5., 0., 0.);
nState.currentVolume = root012.get();
tae.update(tContext, nState);
rvf.update(tContext, nState);
BOOST_CHECK(nState.currentVolume == cyl0.get());
// Cylinder 1
nState.position = Acts::Vector3(50., 0., 0.);
nState.currentVolume = root012.get();
tae.update(tContext, nState);
rvf.update(tContext, nState);
BOOST_CHECK(nState.currentVolume == cyl1.get());
// Cylinder 2
nState.position = Acts::Vector3(150., 0., 0.);
nState.currentVolume = root012.get();
tae.update(tContext, nState);
rvf.update(tContext, nState);
BOOST_CHECK(nState.currentVolume == cyl2.get());

nState.currentVolume = nullptr;
BOOST_CHECK_THROW(tae.update(tContext, nState), std::runtime_error);
nState.currentDetector = nullptr;
BOOST_CHECK_THROW(rvf.update(tContext, nState), std::runtime_error);
}

// Test finding detectors beu trial and error
Expand Down
2 changes: 1 addition & 1 deletion Tests/UnitTests/Core/Navigation/NextNavigatorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ BOOST_AUTO_TEST_CASE(NextNavigator) {
Acts::Experimental::tryAllPortalsAndSurfaces());

auto detector = Acts::Experimental::Detector::makeShared(
"Detector", detectorVolume, Acts::Experimental::tryRootVolume());
"Detector", {detectorVolume}, Acts::Experimental::tryRootVolumes());

using ActionListType = Acts::ActionList<>;
using AbortListType = Acts::AbortList<>;
Expand Down