From 6e0d5b1a8bc228c73c2b6534135fb753d9f0fdc9 Mon Sep 17 00:00:00 2001 From: Andreas Salzburger Date: Wed, 29 Nov 2023 10:07:16 +0100 Subject: [PATCH] feat: respect portal material when fusing portals (#2735) This PR introduces the correct handling of material when fusing portals. The logic is as follows: - if either of the to-be fused surfaces has material, it is being kept in the fusing process - if both have material, an exception is thrown as this certainly indicates misconfiguration A UnitTest is added to check for this behaviour. --- Core/include/Acts/Detector/Portal.hpp | 4 + Core/src/Detector/Portal.cpp | 10 +++ Tests/UnitTests/Core/Detector/PortalTests.cpp | 89 +++++++++++++++++++ 3 files changed, 103 insertions(+) diff --git a/Core/include/Acts/Detector/Portal.hpp b/Core/include/Acts/Detector/Portal.hpp index db8f02caad2..cde49ba00d8 100644 --- a/Core/include/Acts/Detector/Portal.hpp +++ b/Core/include/Acts/Detector/Portal.hpp @@ -116,6 +116,10 @@ class Portal : public std::enable_shared_from_this { /// into this volume, it will throw an exception if the /// portals are not fusable /// + /// @note if one portal carries material, it will be kept, + /// however, if both portals carry material, an exception + /// will be thrown and the portals are not fusable + /// /// @note that other will be overwritten to point to this void fuse(std::shared_ptr& other) noexcept(false); diff --git a/Core/src/Detector/Portal.cpp b/Core/src/Detector/Portal.cpp index 6e4f35d437d..77b67d0633b 100644 --- a/Core/src/Detector/Portal.cpp +++ b/Core/src/Detector/Portal.cpp @@ -87,6 +87,16 @@ void Acts::Experimental::Portal::fuse(std::shared_ptr& other) { "Portal: trying to fuse portal (waste) with no links."); } + if (m_surface->surfaceMaterial() != nullptr && + other->surface().surfaceMaterial() != nullptr) { + throw std::runtime_error( + "Portal: both surfaces have surface material, fusing will lead to " + "information loss."); + } else if (other->surface().surfaceMaterial() != nullptr) { + m_surface->assignSurfaceMaterial( + other->surface().surfaceMaterialSharedPtr()); + } + auto odx = oDir.index(); m_volumeUpdators[odx] = std::move(other->m_volumeUpdators[odx]); m_attachedVolumes[odx] = other->m_attachedVolumes[odx]; diff --git a/Tests/UnitTests/Core/Detector/PortalTests.cpp b/Tests/UnitTests/Core/Detector/PortalTests.cpp index 7d128bffbc2..c7ad9ba2362 100644 --- a/Tests/UnitTests/Core/Detector/PortalTests.cpp +++ b/Tests/UnitTests/Core/Detector/PortalTests.cpp @@ -16,6 +16,8 @@ #include "Acts/Geometry/CuboidVolumeBounds.hpp" #include "Acts/Geometry/GeometryContext.hpp" #include "Acts/Geometry/GeometryIdentifier.hpp" +#include "Acts/Material/HomogeneousSurfaceMaterial.hpp" +#include "Acts/Material/MaterialSlab.hpp" #include "Acts/Navigation/NavigationDelegates.hpp" #include "Acts/Navigation/NavigationState.hpp" #include "Acts/Navigation/SurfaceCandidatesUpdators.hpp" @@ -168,4 +170,91 @@ BOOST_AUTO_TEST_CASE(PortalTest) { BOOST_CHECK_THROW(portalAI->fuse(portalBI), std::runtime_error); } +BOOST_AUTO_TEST_CASE(PortalMaterialTest) { + // Volume A and B + auto dTransform = Acts::Transform3::Identity(); + auto pGenerator = defaultPortalGenerator(); + auto volumeA = DetectorVolumeFactory::construct( + pGenerator, tContext, "dummyA", dTransform, + std::make_unique(1, 1, 1), + tryAllPortalsAndSurfaces()); + auto volumeB = DetectorVolumeFactory::construct( + pGenerator, tContext, "dummyB", dTransform, + std::make_unique(1, 1, 1), + tryAllPortalsAndSurfaces()); + + // Create some material + auto materialSlab = Acts::MaterialSlab( + Acts::Material::fromMolarDensity(1., 2., 3., 4., 5.), 1.); + auto materialA = + std::make_shared(materialSlab); + auto materialB = + std::make_shared(materialSlab); + + // A few portals + auto rectangle = std::make_shared(10., 100.); + + auto surfaceA = Acts::Surface::makeShared( + Acts::Transform3::Identity(), rectangle); + surfaceA->assignSurfaceMaterial(materialA); + auto portalA = Acts::Experimental::Portal::makeShared(surfaceA); + + DetectorVolumeUpdator linkToA; + auto linkToAImpl = std::make_unique(volumeA); + linkToA.connect<&LinkToVolumeImpl::link>(std::move(linkToAImpl)); + portalA->assignDetectorVolumeUpdator(Acts::Direction::Positive, + std::move(linkToA), {volumeA}); + + auto surfaceB = Acts::Surface::makeShared( + Acts::Transform3::Identity(), rectangle); + auto portalB = Acts::Experimental::Portal::makeShared(surfaceB); + DetectorVolumeUpdator linkToB; + auto linkToBImpl = std::make_unique(volumeB); + linkToB.connect<&LinkToVolumeImpl::link>(std::move(linkToBImpl)); + portalB->assignDetectorVolumeUpdator(Acts::Direction::Negative, + std::move(linkToB), {volumeB}); + + // Portal A fuses with B + // - has material and keeps it, portal B becomes portal A + portalA->fuse(portalB); + BOOST_CHECK_EQUAL(portalA->surface().surfaceMaterial(), materialA.get()); + BOOST_CHECK_EQUAL(portalA, portalB); + + // Remake portal B + portalB = Acts::Experimental::Portal::makeShared(surfaceB); + DetectorVolumeUpdator linkToB2; + auto linkToB2Impl = std::make_unique(volumeB); + linkToB2.connect<&LinkToVolumeImpl::link>(std::move(linkToB2Impl)); + portalB->assignDetectorVolumeUpdator(Acts::Direction::Negative, + std::move(linkToB2), {volumeB}); + + // Portal B fuses with A + // - A has material and keeps it, portal B gets it from A, A becomes B + BOOST_REQUIRE_NE(portalA, portalB); + portalB->fuse(portalA); + BOOST_CHECK_EQUAL(portalB->surface().surfaceMaterial(), materialA.get()); + BOOST_CHECK_EQUAL(portalB, portalA); + + // Remake portal A and B, this time both with material + portalA = Acts::Experimental::Portal::makeShared(surfaceA); + DetectorVolumeUpdator linkToA2; + auto linkToA2Impl = std::make_unique(volumeA); + linkToA2.connect<&LinkToVolumeImpl::link>(std::move(linkToA2Impl)); + portalA->assignDetectorVolumeUpdator(Acts::Direction::Positive, + std::move(linkToA2), {volumeA}); + + surfaceB->assignSurfaceMaterial(materialB); + portalB = Acts::Experimental::Portal::makeShared(surfaceB); + DetectorVolumeUpdator linkToB3; + auto linkToB3Impl = std::make_unique(volumeB); + linkToB3.connect<&LinkToVolumeImpl::link>(std::move(linkToB3Impl)); + portalB->assignDetectorVolumeUpdator(Acts::Direction::Negative, + std::move(linkToB3), {volumeB}); + + // Portal A fuses with B - both have material, throw exception + BOOST_CHECK_THROW(portalA->fuse(portalB), std::runtime_error); + // Same in reverse + BOOST_CHECK_THROW(portalB->fuse(portalA), std::runtime_error); +} + BOOST_AUTO_TEST_SUITE_END()