Skip to content

Commit

Permalink
internal storage of volumes becomes mutable
Browse files Browse the repository at this point in the history
this is so we can reassign volumes later on
we need to be careful that this doesn't leak out however
  • Loading branch information
paulgessinger committed Aug 9, 2024
1 parent cc29f3e commit ed3fd39
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 65 deletions.
21 changes: 9 additions & 12 deletions Core/include/Acts/Geometry/GridPortalLink.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
#include "Acts/Utilities/Grid.hpp"
#include "Acts/Utilities/Logger.hpp"

#include <format>
#include <vector>

namespace Acts {

class IGrid;
Expand Down Expand Up @@ -68,16 +65,16 @@ class GridPortalLink : public PortalLinkBase {
}

static std::unique_ptr<GridPortalLink> make(
const std::shared_ptr<RegularSurface>& surface,
const TrackingVolume& volume, BinningValue direction);
const std::shared_ptr<RegularSurface>& surface, TrackingVolume& volume,
BinningValue direction);

static std::unique_ptr<PortalLinkBase> merge(
const std::shared_ptr<GridPortalLink>& a,
const std::shared_ptr<GridPortalLink>& b, BinningValue direction,
const Logger& logger = getDummyLogger());

virtual const IGrid& grid() const = 0;
virtual void setVolume(const TrackingVolume* volume) = 0;
virtual void setVolume(TrackingVolume* volume) = 0;
virtual unsigned int dim() const = 0;
virtual std::unique_ptr<GridPortalLink> make2DGrid(
const IAxis* other) const = 0;
Expand Down Expand Up @@ -115,9 +112,9 @@ class GridPortalLink : public PortalLinkBase {

// These should only be used to synchronize the bins between grids
virtual IndexType numLocalBins() const = 0;
virtual const TrackingVolume*& atLocalBins(IndexType indices) = 0;
virtual TrackingVolume*& atLocalBins(IndexType indices) = 0;

virtual const TrackingVolume* atLocalBins(IndexType indices) const = 0;
virtual TrackingVolume* atLocalBins(IndexType indices) const = 0;

private:
BinningValue m_direction;
Expand All @@ -130,7 +127,7 @@ template <typename... Axes>
requires(sizeof...(Axes) <= 2) class GridPortalLinkT final
: public GridPortalLink {
public:
using GridType = Grid<const TrackingVolume*, Axes...>;
using GridType = Grid<TrackingVolume*, Axes...>;
static constexpr std::size_t DIM = sizeof...(Axes);

GridPortalLinkT(std::shared_ptr<RegularSurface> surface,
Expand Down Expand Up @@ -173,7 +170,7 @@ requires(sizeof...(Axes) <= 2) class GridPortalLinkT final
}
}

void setVolume(const TrackingVolume* volume) final {
void setVolume(TrackingVolume* volume) final {
auto loc = m_grid.numLocalBins();
if constexpr (GridType::DIM == 1) {
for (std::size_t i = 1; i <= loc[0]; i++) {
Expand Down Expand Up @@ -223,7 +220,7 @@ requires(sizeof...(Axes) <= 2) class GridPortalLinkT final
return result;
}

const TrackingVolume*& atLocalBins(IndexType indices) final {
TrackingVolume*& atLocalBins(IndexType indices) final {
throw_assert(indices.size() == DIM, "Invalid number of indices");
typename GridType::index_t idx;
for (std::size_t i = 0; i < DIM; i++) {
Expand All @@ -232,7 +229,7 @@ requires(sizeof...(Axes) <= 2) class GridPortalLinkT final
return m_grid.atLocalBins(idx);
}

const TrackingVolume* atLocalBins(IndexType indices) const final {
TrackingVolume* atLocalBins(IndexType indices) const final {
throw_assert(indices.size() == DIM, "Invalid number of indices");
typename GridType::index_t idx;
for (std::size_t i = 0; i < DIM; i++) {
Expand Down
6 changes: 3 additions & 3 deletions Core/include/Acts/Geometry/TrivialPortalLink.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ class TrackingVolume;
class TrivialPortalLink final : public PortalLinkBase {
public:
TrivialPortalLink(std::shared_ptr<RegularSurface> surface,
const TrackingVolume* volume)
TrackingVolume* volume)
: PortalLinkBase(std::move(surface)), m_volume(volume) {
assert(m_volume != nullptr);
}

TrivialPortalLink(std::shared_ptr<RegularSurface> surface,
const TrackingVolume& volume)
TrackingVolume& volume)
: TrivialPortalLink(std::move(surface), &volume) {}

std::unique_ptr<GridPortalLink> makeGrid(BinningValue direction) const;
Expand All @@ -36,7 +36,7 @@ class TrivialPortalLink final : public PortalLinkBase {
const Vector2& position) const final;

private:
const TrackingVolume* m_volume;
TrackingVolume* m_volume;
};

} // namespace Acts
8 changes: 5 additions & 3 deletions Core/src/Geometry/GridPortalLink.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@
#include "Acts/Surfaces/PlaneSurface.hpp"
#include "Acts/Surfaces/RadialBounds.hpp"

#include <format>

namespace Acts {

std::unique_ptr<GridPortalLink> GridPortalLink::make(
const std::shared_ptr<RegularSurface>& surface,
const TrackingVolume& volume, BinningValue direction) {
const std::shared_ptr<RegularSurface>& surface, TrackingVolume& volume,
BinningValue direction) {
std::unique_ptr<GridPortalLink> grid;

if (const auto* cylinder =
Expand Down Expand Up @@ -276,7 +278,7 @@ void GridPortalLink::fillGrid1dTo2d(FillDirection dir,
assert(locDest.size() == 2);

for (std::size_t i = 0; i <= locSource[0] + 1; ++i) {
const auto* source = grid1d.atLocalBins({i});
TrackingVolume* source = grid1d.atLocalBins({i});

if (dir == FillDirection::loc1) {
for (std::size_t j = 0; j <= locDest[1] + 1; ++j) {
Expand Down
98 changes: 51 additions & 47 deletions Tests/UnitTests/Core/Geometry/PortalTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ struct Fixture {
~Fixture() { Acts::Logging::setFailureThreshold(m_level); }
};

std::shared_ptr<TrackingVolume> makeDummyVolume() {
return std::make_shared<TrackingVolume>(
Transform3::Identity(),
std::make_shared<CylinderVolumeBounds>(30_mm, 40_mm, 100_mm));
}

GeometryContext gctx;

BOOST_FIXTURE_TEST_SUITE(Geometry, Fixture)
Expand Down Expand Up @@ -520,16 +526,15 @@ BOOST_AUTO_TEST_SUITE(Merging1dCylinder)
BOOST_AUTO_TEST_SUITE(ZDirection)

BOOST_AUTO_TEST_CASE(ColinearMerge) {
// WARNING: These are invalid pointers!
const auto* vol1 = reinterpret_cast<const TrackingVolume*>(0x000001);
const auto* vol2 = reinterpret_cast<const TrackingVolume*>(0x000002);
auto vol1 = makeDummyVolume();
auto vol2 = makeDummyVolume();

auto cyl = Surface::makeShared<CylinderSurface>(Transform3::Identity(), 30_mm,
100_mm);

std::shared_ptr grid1dCyl = GridPortalLink::make(
cyl, BinningValue::binZ, Axis{AxisBound, -100_mm, 100_mm, 10});
grid1dCyl->setVolume(vol1);
grid1dCyl->setVolume(vol1.get());

// Another cylinder, shifted in z
auto cyl2 = Surface::makeShared<CylinderSurface>(
Expand All @@ -538,7 +543,7 @@ BOOST_AUTO_TEST_CASE(ColinearMerge) {
std::shared_ptr grid1dCyl2 = GridPortalLink::make(
cyl2, BinningValue::binZ, Axis{AxisBound, -50_mm, 50_mm, 5});

grid1dCyl2->setVolume(vol2);
grid1dCyl2->setVolume(vol2.get());

// Completely invalid
BOOST_CHECK_THROW(GridPortalLink::merge(grid1dCyl, grid1dCyl2,
Expand Down Expand Up @@ -1381,9 +1386,9 @@ BOOST_AUTO_TEST_SUITE_END() // PhiDirection

BOOST_AUTO_TEST_CASE(BinFilling) {
// Volumes for bin content checking
// WARNING: These are invalid pointers!
const auto* vol1 = reinterpret_cast<const TrackingVolume*>(0x000001);
const auto* vol2 = reinterpret_cast<const TrackingVolume*>(0x000002);

auto vol1 = makeDummyVolume();
auto vol2 = makeDummyVolume();

BOOST_TEST_CONTEXT("RDirection") {
auto disc1 = Surface::makeShared<DiscSurface>(Transform3::Identity(), 30_mm,
Expand All @@ -1392,15 +1397,15 @@ BOOST_AUTO_TEST_CASE(BinFilling) {
std::shared_ptr grid1 = GridPortalLink::make(
disc1, BinningValue::binR, Axis{AxisBound, 30_mm, 60_mm, 2});

grid1->setVolume(vol1);
grid1->setVolume(vol1.get());

auto disc2 = Surface::makeShared<DiscSurface>(Transform3::Identity(), 60_mm,
90_mm, 30_degree);

std::shared_ptr grid2 = GridPortalLink::make(
disc2, BinningValue::binR, Axis{AxisBound, 60_mm, 90_mm, 2});

grid2->setVolume(vol2);
grid2->setVolume(vol2.get());

auto mergedPtr =
GridPortalLink::merge(grid1, grid2, BinningValue::binR, *logger);
Expand All @@ -1415,10 +1420,10 @@ BOOST_AUTO_TEST_CASE(BinFilling) {
grid2->printContents(std::cout);
merged->printContents(std::cout);

BOOST_CHECK_EQUAL(merged->grid().atLocalBins({1}), vol1);
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({2}), vol1);
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({3}), vol2);
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({4}), vol2);
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({1}), vol1.get());
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({2}), vol1.get());
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({3}), vol2.get());
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({4}), vol2.get());
}

BOOST_TEST_CONTEXT("PhiDirection") {
Expand All @@ -1428,7 +1433,7 @@ BOOST_AUTO_TEST_CASE(BinFilling) {
std::shared_ptr grid1 = GridPortalLink::make(
disc1, BinningValue::binPhi, Axis{AxisBound, -30_degree, 30_degree, 2});

grid1->setVolume(vol1);
grid1->setVolume(vol1.get());

auto disc2 = Surface::makeShared<DiscSurface>(
Transform3{AngleAxis3{60_degree, Vector3::UnitZ()}}, 30_mm, 100_mm,
Expand All @@ -1437,7 +1442,7 @@ BOOST_AUTO_TEST_CASE(BinFilling) {
std::shared_ptr grid2 = GridPortalLink::make(
disc2, BinningValue::binPhi, Axis{AxisBound, -30_degree, 30_degree, 2});

grid2->setVolume(vol2);
grid2->setVolume(vol2.get());

auto mergedPtr =
GridPortalLink::merge(grid1, grid2, BinningValue::binPhi, *logger);
Expand All @@ -1453,10 +1458,10 @@ BOOST_AUTO_TEST_CASE(BinFilling) {
grid2->printContents(std::cout);
merged->printContents(std::cout);

BOOST_CHECK_EQUAL(merged->grid().atLocalBins({1}), vol2);
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({2}), vol2);
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({3}), vol1);
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({4}), vol1);
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({1}), vol2.get());
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({2}), vol2.get());
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({3}), vol1.get());
BOOST_CHECK_EQUAL(merged->grid().atLocalBins({4}), vol1.get());
}
}

Expand Down Expand Up @@ -1871,28 +1876,27 @@ BOOST_AUTO_TEST_SUITE(MergingCrossDisc)

BOOST_AUTO_TEST_CASE(RDirection) {
// Volumes for bin content checking
// WARNING: These are invalid pointers!
const auto* vol1 = reinterpret_cast<const TrackingVolume*>(0x000001);
const auto* vol2 = reinterpret_cast<const TrackingVolume*>(0x000002);
const auto* vol3 = reinterpret_cast<const TrackingVolume*>(0x000003);
const auto* vol4 = reinterpret_cast<const TrackingVolume*>(0x000004);
auto vol1 = makeDummyVolume();
auto vol2 = makeDummyVolume();
auto vol3 = makeDummyVolume();
auto vol4 = makeDummyVolume();

auto disc1 = Surface::makeShared<DiscSurface>(Transform3::Identity(), 30_mm,
100_mm, 30_degree);

std::shared_ptr grid1 = GridPortalLink::make(
disc1, BinningValue::binR, Axis{AxisBound, 30_mm, 100_mm, 2});
grid1->grid().atLocalBins({1}) = vol1;
grid1->grid().atLocalBins({2}) = vol2;
grid1->grid().atLocalBins({1}) = vol1.get();
grid1->grid().atLocalBins({2}) = vol2.get();

auto disc2 = Surface::makeShared<DiscSurface>(Transform3::Identity(), 100_mm,
150_mm, 30_degree);

std::shared_ptr grid2 = GridPortalLink::make(
disc2, BinningValue::binPhi, Axis{AxisBound, -30_degree, 30_degree, 2});

grid2->grid().atLocalBins({1}) = vol3;
grid2->grid().atLocalBins({2}) = vol4;
grid2->grid().atLocalBins({1}) = vol3.get();
grid2->grid().atLocalBins({2}) = vol4.get();

auto mergedPtr =
GridPortalLink::merge(grid1, grid2, BinningValue::binR, *logger);
Expand All @@ -1908,12 +1912,12 @@ BOOST_AUTO_TEST_CASE(RDirection) {
BOOST_CHECK_EQUAL(axis1, axisExpectedR);
BOOST_CHECK_EQUAL(axis2, axisExpectedPhi);

std::vector<std::pair<Vector2, const TrackingVolume*>> locations = {
{{40_mm, -15_degree}, vol1}, {{40_mm, 15_degree}, vol1},
{{90_mm, -15_degree}, vol2}, {{90_mm, 15_degree}, vol2},
std::vector<std::pair<Vector2, TrackingVolume*>> locations = {
{{40_mm, -15_degree}, vol1.get()}, {{40_mm, 15_degree}, vol1.get()},
{{90_mm, -15_degree}, vol2.get()}, {{90_mm, 15_degree}, vol2.get()},

{{110_mm, -15_degree}, vol3}, {{110_mm, 15_degree}, vol4},
{{140_mm, -15_degree}, vol3}, {{140_mm, 15_degree}, vol4},
{{110_mm, -15_degree}, vol3.get()}, {{110_mm, 15_degree}, vol4.get()},
{{140_mm, -15_degree}, vol3.get()}, {{140_mm, 15_degree}, vol4.get()},
};

for (const auto& [loc, vol] : locations) {
Expand All @@ -1928,20 +1932,20 @@ BOOST_AUTO_TEST_CASE(RDirection) {

BOOST_AUTO_TEST_CASE(PhiDirection) {
// Volumes for bin content checking
// WARNING: These are invalid pointers!
const auto* vol1 = reinterpret_cast<const TrackingVolume*>(0x000001);
const auto* vol2 = reinterpret_cast<const TrackingVolume*>(0x000002);
const auto* vol3 = reinterpret_cast<const TrackingVolume*>(0x000003);
const auto* vol4 = reinterpret_cast<const TrackingVolume*>(0x000004);

auto vol1 = makeDummyVolume();
auto vol2 = makeDummyVolume();
auto vol3 = makeDummyVolume();
auto vol4 = makeDummyVolume();

auto disc1 = Surface::makeShared<DiscSurface>(Transform3::Identity(), 30_mm,
100_mm, 30_degree);

std::shared_ptr grid1 = GridPortalLink::make(
disc1, BinningValue::binR, Axis{AxisBound, 30_mm, 100_mm, 2});

grid1->grid().atLocalBins({1}) = vol1;
grid1->grid().atLocalBins({2}) = vol2;
grid1->grid().atLocalBins({1}) = vol1.get();
grid1->grid().atLocalBins({2}) = vol2.get();

auto disc2 = Surface::makeShared<DiscSurface>(
Transform3{AngleAxis3{90_degree, Vector3::UnitZ()}}, 30_mm, 100_mm,
Expand All @@ -1950,8 +1954,8 @@ BOOST_AUTO_TEST_CASE(PhiDirection) {
std::shared_ptr grid2 = GridPortalLink::make(
disc2, BinningValue::binPhi, Axis{AxisBound, -60_degree, 60_degree, 2});

grid2->grid().atLocalBins({1}) = vol3;
grid2->grid().atLocalBins({2}) = vol4;
grid2->grid().atLocalBins({1}) = vol3.get();
grid2->grid().atLocalBins({2}) = vol4.get();

auto mergedPtr =
GridPortalLink::merge(grid1, grid2, BinningValue::binPhi, *logger);
Expand All @@ -1975,10 +1979,10 @@ BOOST_AUTO_TEST_CASE(PhiDirection) {
BOOST_CHECK_EQUAL(axis2.getType(), AxisType::Equidistant);
BOOST_CHECK_EQUAL(axis2.getBoundaryType(), AxisBoundaryType::Bound);

std::vector<std::pair<Vector2, const TrackingVolume*>> locations = {
{{40_mm, 45_degree}, vol1}, {{40_mm, 0_degree}, vol4},
{{40_mm, -80_degree}, vol3}, {{90_mm, 45_degree}, vol2},
{{90_mm, 0_degree}, vol4}, {{90_mm, -80_degree}, vol3},
std::vector<std::pair<Vector2, TrackingVolume*>> locations = {
{{40_mm, 45_degree}, vol1.get()}, {{40_mm, 0_degree}, vol4.get()},
{{40_mm, -80_degree}, vol3.get()}, {{90_mm, 45_degree}, vol2.get()},
{{90_mm, 0_degree}, vol4.get()}, {{90_mm, -80_degree}, vol3.get()},
};

grid1->printContents(std::cout);
Expand Down

0 comments on commit ed3fd39

Please sign in to comment.