Skip to content

Commit

Permalink
fix(geo): CylVolStack reuses gaps if exist (#3716)
Browse files Browse the repository at this point in the history 
This PR makes it such that when a `CylinderVolumeStack` is resized with
the gap strategy, if there are already gaps on the outside of the stack,
**they are reused** instead of creating extra gaps.

Part of to #3502.
     
```                               
  original size
◀───────────────▶
┌───────────────┐
│               │
│               │
│   Volume 1    │
│               │
│               │
└───────────────┘
        first resize
◀──────────────────────────▶
┌───────────────┬──────────┐
│               │          │
│               │          │
│   Volume 1    │   Gap    │
│               │          │      Gap is
│               │          │      reused!──┐
└───────────────┴──────────┘               │
            second resize                  │
◀───────────────────────────────────▶      │
┌───────────────┬───────────────────┐      │
│               │                   │      │
│               │                   │      │
│   Volume 1    │        Gap        │◀─────┘
│               │                   │
│               │                   │
└───────────────┴───────────────────┘
```

Blocked by:
- #3715
  • Loading branch information
@paulgessinger
paulgessinger authored Oct 12, 2024
1 parent 9bd074c commit 010e397
Show file tree
Hide file tree
Showing 2 changed files with 302 additions and 46 deletions.
164 changes: 118 additions & 46 deletions Core/src/Geometry/CylinderVolumeStack.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,11 @@ void CylinderVolumeStack::update(std::shared_ptr<VolumeBounds> volbounds,
throw std::invalid_argument("Shrinking the stack is r in not supported");
}

auto isGap = [this](const Volume* vol) {
return std::ranges::any_of(
m_gaps, [&](const auto& gap) { return vol == gap.get(); });
};

if (m_direction == BinningValue::binZ) {
ACTS_VERBOSE("Stack direction is z");

Expand Down Expand Up @@ -822,43 +827,86 @@ void CylinderVolumeStack::update(std::shared_ptr<VolumeBounds> volbounds,
} else if (m_resizeStrategy == ResizeStrategy::Gap) {
ACTS_VERBOSE("Creating gap volumes to fill the new z bounds");

if (!same(newMinZ, oldMinZ) && newMinZ < oldMinZ) {
ACTS_VERBOSE("Adding gap volume at negative z");
auto printGapDimensions = [&](const VolumeTuple& gap,
const std::string& prefix = "") {
ACTS_VERBOSE(" -> gap" << prefix << ": [ " << gap.minZ() << " <- "
<< gap.midZ() << " -> " << gap.maxZ()
<< " ], r: [ " << gap.minR() << " <-> "
<< gap.maxR() << " ]");
};

if (!same(newMinZ, oldMinZ) && newMinZ < oldMinZ) {
ActsScalar gap1MinZ = newVolume.minZ();
ActsScalar gap1MaxZ = oldVolume.minZ();
ActsScalar gap1HlZ = (gap1MaxZ - gap1MinZ) / 2.0;
ActsScalar gap1PZ = (gap1MaxZ + gap1MinZ) / 2.0;

ACTS_VERBOSE(" -> gap1 z: [ " << gap1MinZ << " <- " << gap1PZ
<< " -> " << gap1MaxZ
<< " ] (hl: " << gap1HlZ << ")");

auto gap1Bounds =
std::make_shared<CylinderVolumeBounds>(newMinR, newMaxR, gap1HlZ);
auto gap1Transform = m_groupTransform * Translation3{0, 0, gap1PZ};
auto gap1 = addGapVolume(gap1Transform, std::move(gap1Bounds));
volumeTuples.insert(volumeTuples.begin(),
VolumeTuple{*gap1, m_groupTransform});
// // check if we need a new gap volume or reuse an existing one
auto& candidate = volumeTuples.front();
if (isGap(candidate.volume)) {
ACTS_VERBOSE("~> Reusing existing gap volume at negative z");

gap1HlZ =
candidate.bounds->get(CylinderVolumeBounds::eHalfLengthZ) +
gap1HlZ;
gap1MaxZ = gap1MinZ + gap1HlZ * 2;
gap1PZ = (gap1MaxZ + gap1MinZ) / 2.0;

printGapDimensions(candidate, " before");
auto gap1Bounds = std::make_shared<CylinderVolumeBounds>(
newMinR, newMaxR, gap1HlZ);
auto gap1Transform = m_groupTransform * Translation3{0, 0, gap1PZ};
candidate.volume->update(std::move(gap1Bounds), gap1Transform);
candidate = VolumeTuple{*candidate.volume, m_groupTransform};
ACTS_VERBOSE("After:");
printGapDimensions(candidate, " after ");

} else {
ACTS_VERBOSE("~> Creating new gap volume at negative z");
auto gap1Bounds = std::make_shared<CylinderVolumeBounds>(
newMinR, newMaxR, gap1HlZ);
auto gap1Transform = m_groupTransform * Translation3{0, 0, gap1PZ};
auto gap1 = addGapVolume(gap1Transform, std::move(gap1Bounds));
volumeTuples.insert(volumeTuples.begin(),
VolumeTuple{*gap1, m_groupTransform});
printGapDimensions(volumeTuples.front());
}
}

if (!same(newMaxZ, oldMaxZ) && newMaxZ > oldMaxZ) {
ACTS_VERBOSE("Adding gap volume at positive z");

ActsScalar gap2MinZ = oldVolume.maxZ();
ActsScalar gap2MaxZ = newVolume.maxZ();
ActsScalar gap2HlZ = (gap2MaxZ - gap2MinZ) / 2.0;
ActsScalar gap2PZ = (gap2MaxZ + gap2MinZ) / 2.0;

ACTS_VERBOSE(" -> gap2 z: [ " << gap2MinZ << " <- " << gap2PZ
<< " -> " << gap2MaxZ
<< " ] (hl: " << gap2HlZ << ")");

auto gap2Bounds =
std::make_shared<CylinderVolumeBounds>(newMinR, newMaxR, gap2HlZ);
auto gap2Transform = m_groupTransform * Translation3{0, 0, gap2PZ};
auto gap2 = addGapVolume(gap2Transform, std::move(gap2Bounds));
volumeTuples.emplace_back(*gap2, m_groupTransform);
// check if we need a new gap volume or reuse an existing one
auto& candidate = volumeTuples.back();
if (isGap(candidate.volume)) {
ACTS_VERBOSE("~> Reusing existing gap volume at positive z");

gap2HlZ =
candidate.bounds->get(CylinderVolumeBounds::eHalfLengthZ) +
gap2HlZ;
gap2MinZ = newVolume.maxZ() - gap2HlZ * 2;
gap2PZ = (gap2MaxZ + gap2MinZ) / 2.0;

printGapDimensions(candidate, " before");
auto gap2Bounds = std::make_shared<CylinderVolumeBounds>(
newMinR, newMaxR, gap2HlZ);
auto gap2Transform = m_groupTransform * Translation3{0, 0, gap2PZ};

candidate.volume->update(std::move(gap2Bounds), gap2Transform);
candidate = VolumeTuple{*candidate.volume, m_groupTransform};
printGapDimensions(candidate, " after ");
} else {
ACTS_VERBOSE("~> Creating new gap volume at positive z");
auto gap2Bounds = std::make_shared<CylinderVolumeBounds>(
newMinR, newMaxR, gap2HlZ);
auto gap2Transform = m_groupTransform * Translation3{0, 0, gap2PZ};
auto gap2 = addGapVolume(gap2Transform, std::move(gap2Bounds));
volumeTuples.emplace_back(*gap2, m_groupTransform);
printGapDimensions(volumeTuples.back());
}
}
}

Expand Down Expand Up @@ -925,32 +973,56 @@ void CylinderVolumeStack::update(std::shared_ptr<VolumeBounds> volbounds,
<< " ]");
}
} else if (m_resizeStrategy == ResizeStrategy::Gap) {
auto printGapDimensions = [&](const VolumeTuple& gap,
const std::string& prefix = "") {
ACTS_VERBOSE(" -> gap" << prefix << ": [ " << gap.minZ() << " <- "
<< gap.midZ() << " -> " << gap.maxZ()
<< " ], r: [ " << gap.minR() << " <-> "
<< gap.maxR() << " ]");
};

if (oldMinR > newMinR) {
ACTS_VERBOSE("Creating gap volume at the innermost end");
auto gapBounds =
std::make_shared<CylinderVolumeBounds>(newMinR, oldMinR, newHlZ);
auto gapTransform = m_groupTransform;
auto gapVolume = addGapVolume(gapTransform, gapBounds);
volumeTuples.insert(volumeTuples.begin(),
VolumeTuple{*gapVolume, m_groupTransform});
auto gap = volumeTuples.front();
ACTS_VERBOSE(" -> gap inner: [ "
<< gap.minZ() << " <- " << gap.midZ() << " -> "
<< gap.maxZ() << " ], r: [ " << gap.minR() << " <-> "
<< gap.maxR() << " ]");
auto& candidate = volumeTuples.front();
if (isGap(candidate.volume)) {
ACTS_VERBOSE("~> Reusing existing gap volume at inner r");
auto& candidateCylBounds = dynamic_cast<CylinderVolumeBounds&>(
candidate.volume->volumeBounds());
printGapDimensions(candidate, " before");
candidateCylBounds.set(CylinderVolumeBounds::eMinR, newMinR);
candidate = VolumeTuple{*candidate.volume, m_groupTransform};
printGapDimensions(candidate, " after ");
} else {
ACTS_VERBOSE("~> Creating new gap volume at inner r");
auto gapBounds = std::make_shared<CylinderVolumeBounds>(
newMinR, oldMinR, newHlZ);
auto gapTransform = m_groupTransform;
auto gapVolume = addGapVolume(gapTransform, gapBounds);
volumeTuples.insert(volumeTuples.begin(),
VolumeTuple{*gapVolume, m_groupTransform});
auto gap = volumeTuples.front();
printGapDimensions(gap);
}
}
if (oldMaxR < newMaxR) {
ACTS_VERBOSE("Creating gap volume at the outermost end");
auto gapBounds =
std::make_shared<CylinderVolumeBounds>(oldMaxR, newMaxR, newHlZ);
auto gapTransform = m_groupTransform;
auto gapVolume = addGapVolume(gapTransform, gapBounds);
volumeTuples.emplace_back(*gapVolume, m_groupTransform);
auto gap = volumeTuples.back();
ACTS_VERBOSE(" -> gap outer: [ "
<< gap.minZ() << " <- " << gap.midZ() << " -> "
<< gap.maxZ() << " ], r: [ " << gap.minR() << " <-> "
<< gap.maxR() << " ]");
auto& candidate = volumeTuples.back();
if (isGap(candidate.volume)) {
ACTS_VERBOSE("~> Reusing existing gap volume at outer r");
auto& candidateCylBounds = dynamic_cast<CylinderVolumeBounds&>(
candidate.volume->volumeBounds());
printGapDimensions(candidate, " before");
candidateCylBounds.set(CylinderVolumeBounds::eMaxR, newMaxR);
candidate = VolumeTuple{*candidate.volume, m_groupTransform};
printGapDimensions(candidate, " after ");
} else {
ACTS_VERBOSE("~> Creating new gap volume at outer r");
auto gapBounds = std::make_shared<CylinderVolumeBounds>(
oldMaxR, newMaxR, newHlZ);
auto gapTransform = m_groupTransform;
auto gapVolume = addGapVolume(gapTransform, gapBounds);
volumeTuples.emplace_back(*gapVolume, m_groupTransform);
auto gap = volumeTuples.back();
printGapDimensions(gap);
}
}
}

Expand Down
184 changes: 184 additions & 0 deletions Tests/UnitTests/Core/Geometry/CylinderVolumeStackTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,114 @@ BOOST_AUTO_TEST_CASE(ResizeReproduction2) {
trf3, *logger);
}

// original size
// <--------------->
// +---------------+
// | |
// | |
// | Volume 1 |
// | |
// | |
// +---------------+
// first resize
// <-------------------------->
// +---------------+----------+
// | | |
// | | |
// | Volume 1 | Gap |
// | | | Gap is
// | | | reused!--+
// +---------------+----------+ |
// second resize |
// <-----------------------------------> |
// +---------------+-------------------+ |
// | | | |
// | | | |
// | Volume 1 | Gap |<-----+
// | | |
// | | |
// +---------------+-------------------+
//
BOOST_AUTO_TEST_CASE(ResizeGapMultiple) {
Transform3 trf = Transform3::Identity();
auto bounds = std::make_shared<CylinderVolumeBounds>(70, 100, 100.0);
Volume vol{trf, bounds};

BOOST_TEST_CONTEXT("Positive") {
std::vector<Volume*> volumes = {&vol};
CylinderVolumeStack stack(volumes, BinningValue::binZ,
CylinderVolumeStack::AttachmentStrategy::Gap,
CylinderVolumeStack::ResizeStrategy::Gap,
*logger);

BOOST_CHECK_EQUAL(volumes.size(), 1);
BOOST_CHECK(stack.gaps().empty());

stack.update(std::make_shared<CylinderVolumeBounds>(30.0, 100, 200),
trf * Translation3{Vector3::UnitZ() * 100}, *logger);
BOOST_CHECK_EQUAL(volumes.size(), 2);
BOOST_CHECK_EQUAL(stack.gaps().size(), 1);

BOOST_CHECK_EQUAL(stack.gaps().front()->center()[eZ], 200.0);
const auto* cylBounds = dynamic_cast<const CylinderVolumeBounds*>(
&stack.gaps().front()->volumeBounds());
BOOST_REQUIRE_NE(cylBounds, nullptr);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eHalfLengthZ),
100.0);

stack.update(std::make_shared<CylinderVolumeBounds>(30.0, 100, 300),
trf * Translation3{Vector3::UnitZ() * 200}, *logger);

BOOST_CHECK_EQUAL(volumes.size(), 2);
// No additional gap volume was added!
BOOST_CHECK_EQUAL(stack.gaps().size(), 1);

BOOST_CHECK_EQUAL(stack.gaps().front()->center()[eZ], 300.0);
cylBounds = dynamic_cast<const CylinderVolumeBounds*>(
&stack.gaps().front()->volumeBounds());
BOOST_REQUIRE_NE(cylBounds, nullptr);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eHalfLengthZ),
200.0);
}

BOOST_TEST_CONTEXT("Negative") {
std::vector<Volume*> volumes = {&vol};
CylinderVolumeStack stack(volumes, BinningValue::binZ,
CylinderVolumeStack::AttachmentStrategy::Gap,
CylinderVolumeStack::ResizeStrategy::Gap,
*logger);

BOOST_CHECK_EQUAL(volumes.size(), 1);
BOOST_CHECK(stack.gaps().empty());

stack.update(std::make_shared<CylinderVolumeBounds>(30.0, 100, 200),
trf * Translation3{Vector3::UnitZ() * -100}, *logger);
BOOST_CHECK_EQUAL(volumes.size(), 2);
BOOST_CHECK_EQUAL(stack.gaps().size(), 1);

BOOST_CHECK_EQUAL(stack.gaps().front()->center()[eZ], -200.0);
const auto* cylBounds = dynamic_cast<const CylinderVolumeBounds*>(
&stack.gaps().front()->volumeBounds());
BOOST_REQUIRE_NE(cylBounds, nullptr);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eHalfLengthZ),
100.0);

stack.update(std::make_shared<CylinderVolumeBounds>(30.0, 100, 300),
trf * Translation3{Vector3::UnitZ() * -200}, *logger);

BOOST_CHECK_EQUAL(volumes.size(), 2);
// No additional gap volume was added!
BOOST_CHECK_EQUAL(stack.gaps().size(), 1);

BOOST_CHECK_EQUAL(stack.gaps().front()->center()[eZ], -300.0);
cylBounds = dynamic_cast<const CylinderVolumeBounds*>(
&stack.gaps().front()->volumeBounds());
BOOST_REQUIRE_NE(cylBounds, nullptr);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eHalfLengthZ),
200.0);
}
}

BOOST_AUTO_TEST_SUITE_END()

BOOST_AUTO_TEST_SUITE(RDirection)
Expand Down Expand Up @@ -1554,6 +1662,82 @@ BOOST_DATA_TEST_CASE(
}
}

BOOST_AUTO_TEST_CASE(ResizeGapMultiple) {
Transform3 trf = Transform3::Identity();
auto bounds = std::make_shared<CylinderVolumeBounds>(100, 200, 100);
Volume vol{trf, bounds};

BOOST_TEST_CONTEXT("Outer") {
std::vector<Volume*> volumes = {&vol};
CylinderVolumeStack stack(volumes, BinningValue::binR,
CylinderVolumeStack::AttachmentStrategy::Gap,
CylinderVolumeStack::ResizeStrategy::Gap,
*logger);

BOOST_CHECK_EQUAL(volumes.size(), 1);
BOOST_CHECK(stack.gaps().empty());

stack.update(std::make_shared<CylinderVolumeBounds>(100, 250, 100), trf,
*logger);
BOOST_CHECK_EQUAL(volumes.size(), 2);
BOOST_CHECK_EQUAL(stack.gaps().size(), 1);

const auto* cylBounds = dynamic_cast<const CylinderVolumeBounds*>(
&stack.gaps().front()->volumeBounds());
BOOST_REQUIRE_NE(cylBounds, nullptr);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eMinR), 200);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eMaxR), 250);

stack.update(std::make_shared<CylinderVolumeBounds>(100, 300, 100), trf,
*logger);

BOOST_CHECK_EQUAL(volumes.size(), 2);
// No additional gap volume was added!
BOOST_CHECK_EQUAL(stack.gaps().size(), 1);

cylBounds = dynamic_cast<const CylinderVolumeBounds*>(
&stack.gaps().front()->volumeBounds());
BOOST_REQUIRE_NE(cylBounds, nullptr);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eMinR), 200);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eMaxR), 300);
}

BOOST_TEST_CONTEXT("Inner") {
std::vector<Volume*> volumes = {&vol};
CylinderVolumeStack stack(volumes, BinningValue::binR,
CylinderVolumeStack::AttachmentStrategy::Gap,
CylinderVolumeStack::ResizeStrategy::Gap,
*logger);

BOOST_CHECK_EQUAL(volumes.size(), 1);
BOOST_CHECK(stack.gaps().empty());

stack.update(std::make_shared<CylinderVolumeBounds>(50, 200, 100), trf,
*logger);
BOOST_CHECK_EQUAL(volumes.size(), 2);
BOOST_CHECK_EQUAL(stack.gaps().size(), 1);

const auto* cylBounds = dynamic_cast<const CylinderVolumeBounds*>(
&stack.gaps().front()->volumeBounds());
BOOST_REQUIRE_NE(cylBounds, nullptr);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eMinR), 50);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eMaxR), 100);

stack.update(std::make_shared<CylinderVolumeBounds>(0, 200, 100), trf,
*logger);

BOOST_CHECK_EQUAL(volumes.size(), 2);
// No additional gap volume was added!
BOOST_CHECK_EQUAL(stack.gaps().size(), 1);

cylBounds = dynamic_cast<const CylinderVolumeBounds*>(
&stack.gaps().front()->volumeBounds());
BOOST_REQUIRE_NE(cylBounds, nullptr);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eMinR), 0);
BOOST_CHECK_EQUAL(cylBounds->get(CylinderVolumeBounds::eMaxR), 100);
}
}

BOOST_AUTO_TEST_SUITE_END()

BOOST_AUTO_TEST_SUITE(Common)
Expand Down

0 comments on commit 010e397

Please sign in to comment.