diff --git a/Core/include/Acts/Detector/DetectorVolume.hpp b/Core/include/Acts/Detector/DetectorVolume.hpp index d16a06ca9cc..c0e6a91fb8c 100644 --- a/Core/include/Acts/Detector/DetectorVolume.hpp +++ b/Core/include/Acts/Detector/DetectorVolume.hpp @@ -484,6 +484,9 @@ class DetectorVolume : public std::enable_shared_from_this { /// @brief A detector volume factory which first constructs the detector volume /// and then constructs the portals. This ensures that the std::shared_ptr /// holding the detector volume is not weak when assigning to the portals. +/// +/// @note Optional containment check is invoked by setting the number +/// of segments nSeg to be greater than 0 class DetectorVolumeFactory { public: /// Create a detector volume - from factory @@ -494,11 +497,19 @@ class DetectorVolumeFactory { const std::vector>& surfaces, const std::vector>& volumes, DetectorVolumeUpdater detectorVolumeUpdater, - SurfaceCandidatesUpdater surfaceCandidateUpdater) { + SurfaceCandidatesUpdater surfaceCandidateUpdater, int nSeg = -1) { auto dVolume = DetectorVolume::makeShared( gctx, name, transform, std::move(bounds), surfaces, volumes, std::move(detectorVolumeUpdater), std::move(surfaceCandidateUpdater)); dVolume->construct(gctx, portalGenerator); + + /// Volume extent is constructed from the portals + /// So the surface/subvolume containment + /// check has to happen here + if (nSeg > 0 && !dVolume->checkContainment(gctx, nSeg)) { + throw std::invalid_argument( + "DetectorVolume: surfaces or subvolumes are not contained by volume"); + } return dVolume; } diff --git a/Core/include/Acts/Geometry/CuboidVolumeBounds.hpp b/Core/include/Acts/Geometry/CuboidVolumeBounds.hpp index 52f8697688b..a56636cc06b 100644 --- a/Core/include/Acts/Geometry/CuboidVolumeBounds.hpp +++ b/Core/include/Acts/Geometry/CuboidVolumeBounds.hpp @@ -122,6 +122,14 @@ class CuboidVolumeBounds : public VolumeBounds { const Vector3& envelope = {0, 0, 0}, const Volume* entity = nullptr) const final; + /// Get the canonical binning values, i.e. the binning values + /// for that fully describe the shape's extent + /// + /// @return vector of canonical binning values + std::vector canonicalBinning() const override { + return {Acts::binX, Acts::binY, Acts::binZ}; + }; + /// Binning borders in double /// /// @param bValue is the binning schema used diff --git a/Core/include/Acts/Geometry/CutoutCylinderVolumeBounds.hpp b/Core/include/Acts/Geometry/CutoutCylinderVolumeBounds.hpp index 52031d2514f..3033b83595e 100644 --- a/Core/include/Acts/Geometry/CutoutCylinderVolumeBounds.hpp +++ b/Core/include/Acts/Geometry/CutoutCylinderVolumeBounds.hpp @@ -114,6 +114,14 @@ class CutoutCylinderVolumeBounds : public VolumeBounds { const Vector3& envelope = {0, 0, 0}, const Volume* entity = nullptr) const final; + /// Get the canonical binning values, i.e. the binning values + /// for that fully describe the shape's extent + /// + /// @return vector of canonical binning values + std::vector canonicalBinning() const override { + return {Acts::binR, Acts::binPhi, Acts::binZ}; + }; + /// Write information about this instance to an outstream /// /// @param sl The outstream diff --git a/Core/include/Acts/Geometry/CylinderVolumeBounds.hpp b/Core/include/Acts/Geometry/CylinderVolumeBounds.hpp index 4f8eb9dddca..c6f60f555cf 100644 --- a/Core/include/Acts/Geometry/CylinderVolumeBounds.hpp +++ b/Core/include/Acts/Geometry/CylinderVolumeBounds.hpp @@ -172,6 +172,14 @@ class CylinderVolumeBounds : public VolumeBounds { const Vector3& envelope = {0, 0, 0}, const Volume* entity = nullptr) const final; + /// Get the canonical binning values, i.e. the binning values + /// for that fully describe the shape's extent + /// + /// @return vector of canonical binning values + std::vector canonicalBinning() const override { + return {Acts::binR, Acts::binPhi, Acts::binZ}; + }; + /// Binning offset - overloaded for some R-binning types /// /// @param bValue is the type used for the binning diff --git a/Core/include/Acts/Geometry/GenericCuboidVolumeBounds.hpp b/Core/include/Acts/Geometry/GenericCuboidVolumeBounds.hpp index d205f51d92a..138ab0d7692 100644 --- a/Core/include/Acts/Geometry/GenericCuboidVolumeBounds.hpp +++ b/Core/include/Acts/Geometry/GenericCuboidVolumeBounds.hpp @@ -89,6 +89,14 @@ class GenericCuboidVolumeBounds : public VolumeBounds { const Vector3& envelope = {0, 0, 0}, const Volume* entity = nullptr) const final; + /// Get the canonical binning values, i.e. the binning values + /// for that fully describe the shape's extent + /// + /// @return vector of canonical binning values + std::vector canonicalBinning() const override { + return {Acts::binX, Acts::binY, Acts::binZ}; + }; + /// @param sl is the output stream to be written into std::ostream& toStream(std::ostream& sl) const override; diff --git a/Core/include/Acts/Geometry/VolumeBounds.hpp b/Core/include/Acts/Geometry/VolumeBounds.hpp index 860977172f7..93cb16a15a5 100644 --- a/Core/include/Acts/Geometry/VolumeBounds.hpp +++ b/Core/include/Acts/Geometry/VolumeBounds.hpp @@ -115,6 +115,18 @@ class VolumeBounds { const Transform3* trf = nullptr, const Vector3& envelope = {0, 0, 0}, const Volume* entity = nullptr) const = 0; + /// Get the canonical binning values, i.e. the binning values + /// for that fully describe the shape's extent + /// + /// @return vector of canonical binning values + /// + /// @note This is the default implementation that + /// returns the bounding box binning. Individual shapes + /// should override this method + virtual std::vector canonicalBinning() const { + return {Acts::binX, Acts::binY, Acts::binZ}; + }; + /// Binning offset - overloaded for some R-binning types /// /// @param bValue is the binning schema used diff --git a/Core/src/Detector/DetectorVolume.cpp b/Core/src/Detector/DetectorVolume.cpp index c233f36ae3b..c0edaf2d11d 100644 --- a/Core/src/Detector/DetectorVolume.cpp +++ b/Core/src/Detector/DetectorVolume.cpp @@ -55,7 +55,6 @@ Acts::Experimental::DetectorVolume::DetectorVolume( } [[maybe_unused]] const auto& gctx_ref = gctx; - assert(checkContainment(gctx) && "Objects are not contained by volume."); } Acts::Experimental::DetectorVolume::DetectorVolume( @@ -264,20 +263,27 @@ Acts::Extent Acts::Experimental::DetectorVolume::extent( bool Acts::Experimental::DetectorVolume::checkContainment( const GeometryContext& gctx, std::size_t nseg) const { + // We don't have a logging instance here + // so can't throw a warning for shapes that are + // using the bounding box + auto binningValues = volumeBounds().canonicalBinning(); + // Create the volume extent auto volumeExtent = extent(gctx, nseg); // Check surfaces - for (const auto* s : surfaces()) { - auto sExtent = s->polyhedronRepresentation(gctx, nseg).extent(); - if (!volumeExtent.contains(sExtent)) { - return false; + for (auto b : binningValues) { + for (const auto* s : surfaces()) { + auto sExtent = s->polyhedronRepresentation(gctx, nseg).extent(); + if (!volumeExtent.contains(sExtent, b)) { + return false; + } } - } - // Check volumes - for (const auto* v : volumes()) { - auto vExtent = v->extent(gctx, nseg); - if (!volumeExtent.contains(vExtent)) { - return false; + // Check volumes + for (const auto* v : volumes()) { + auto vExtent = v->extent(gctx, nseg); + if (!volumeExtent.contains(vExtent, b)) { + return false; + } } } // All contained diff --git a/Plugins/Json/include/Acts/Plugins/Json/AlgebraJsonConverter.hpp b/Plugins/Json/include/Acts/Plugins/Json/AlgebraJsonConverter.hpp index b54cef52eca..b93a8703795 100644 --- a/Plugins/Json/include/Acts/Plugins/Json/AlgebraJsonConverter.hpp +++ b/Plugins/Json/include/Acts/Plugins/Json/AlgebraJsonConverter.hpp @@ -28,7 +28,7 @@ struct Options { /// Write the identity transform explicitly bool writeIdentity = false; /// Apply a transpose to flip column/row order - bool transpose = true; + bool transpose = false; }; /// @brief The Transform converter to json diff --git a/Tests/UnitTests/Core/Detector/DetectorVolumeTests.cpp b/Tests/UnitTests/Core/Detector/DetectorVolumeTests.cpp index 91b861a1f43..c93ebd345e8 100644 --- a/Tests/UnitTests/Core/Detector/DetectorVolumeTests.cpp +++ b/Tests/UnitTests/Core/Detector/DetectorVolumeTests.cpp @@ -25,6 +25,8 @@ #include "Acts/Navigation/SurfaceCandidatesUpdaters.hpp" #include "Acts/Surfaces/CylinderBounds.hpp" #include "Acts/Surfaces/CylinderSurface.hpp" +#include "Acts/Surfaces/PlaneSurface.hpp" +#include "Acts/Surfaces/RectangleBounds.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Tests/CommonHelpers/FloatComparisons.hpp" #include "Acts/Utilities/BinningType.hpp" @@ -53,6 +55,75 @@ Acts::GeometryContext tContext; BOOST_AUTO_TEST_SUITE(Detector) +BOOST_AUTO_TEST_CASE(SurfaceVolumeContainment) { + // Create a surface that is placed way off + auto surfaceOutOfBounds = Acts::Surface::makeShared( + Acts::Transform3::Identity() * Acts::Translation3(-1000, 0., 0.), + std::make_shared(1, 1)); + + auto vBounds = Acts::CuboidVolumeBounds(10.0, 10.0, 10.0); + auto portalGenerator = + Acts::Experimental::defaultPortalAndSubPortalGenerator(); + BOOST_CHECK_THROW( + Acts::Experimental::DetectorVolumeFactory::construct( + portalGenerator, Acts::GeometryContext(), + "CubeWithOutofBoundsSurface", Acts::Transform3::Identity(), + std::make_shared(vBounds), + {surfaceOutOfBounds}, {}, Acts::Experimental::tryAllSubVolumes(), + Acts::Experimental::tryAllPortalsAndSurfaces(), 1000), + std::invalid_argument); + + // Create a surface that is too big + auto surfaceTooBig = Acts::Surface::makeShared( + Acts::Transform3::Identity() * Acts::Translation3(0, 0., 0.), + std::make_shared(1, 100)); + + BOOST_CHECK_THROW( + Acts::Experimental::DetectorVolumeFactory::construct( + portalGenerator, Acts::GeometryContext(), "CubeWithSurfaceTooBig", + Acts::Transform3::Identity(), + std::make_shared(vBounds), {surfaceTooBig}, + {}, Acts::Experimental::tryAllSubVolumes(), + Acts::Experimental::tryAllPortalsAndSurfaces(), 1000), + std::invalid_argument); + + // Envelope a bigger volume into a smaller one + auto bigVolume = Acts::Experimental::DetectorVolumeFactory::construct( + portalGenerator, Acts::GeometryContext(), "BigCube", + Acts::Transform3::Identity(), + std::make_shared(vBounds), {}, {}, + Acts::Experimental::tryAllSubVolumes(), + Acts::Experimental::tryAllPortalsAndSurfaces(), 1000); + + auto smallBounds = Acts::CuboidVolumeBounds(1.0, 1.0, 1.0); + BOOST_CHECK_THROW( + Acts::Experimental::DetectorVolumeFactory::construct( + portalGenerator, Acts::GeometryContext(), + "SmallCubeWithBigCubeInside", Acts::Transform3::Identity(), + std::make_shared(smallBounds), {}, + {bigVolume}, Acts::Experimental::tryAllSubVolumes(), + Acts::Experimental::tryAllPortalsAndSurfaces(), 1000), + std::invalid_argument); + + // Envelope a misaligned subvolume + auto smallVolumeMisaligned = + Acts::Experimental::DetectorVolumeFactory::construct( + portalGenerator, Acts::GeometryContext(), "SmallCubeMisaligned", + Acts::Transform3::Identity() * Acts::Translation3(9.5, 0., 0.), + std::make_shared(smallBounds), {}, {}, + Acts::Experimental::tryAllSubVolumes(), + Acts::Experimental::tryAllPortalsAndSurfaces(), 1000); + + BOOST_CHECK_THROW( + Acts::Experimental::DetectorVolumeFactory::construct( + portalGenerator, Acts::GeometryContext(), "CubeWithMisalignedVolume", + Acts::Transform3::Identity(), + std::make_shared(vBounds), {}, + {smallVolumeMisaligned}, Acts::Experimental::tryAllSubVolumes(), + Acts::Experimental::tryAllPortalsAndSurfaces(), 1000), + std::invalid_argument); +} + BOOST_AUTO_TEST_CASE(CylindricalDetectorVolumePortals) { Acts::ActsScalar rInner = 10.; Acts::ActsScalar rOuter = 100.; diff --git a/Tests/UnitTests/Plugins/DD4hep/DD4hepCylindricalDetectorTests.cpp b/Tests/UnitTests/Plugins/DD4hep/DD4hepCylindricalDetectorTests.cpp index a565bad3dca..894bfb89cce 100644 --- a/Tests/UnitTests/Plugins/DD4hep/DD4hepCylindricalDetectorTests.cpp +++ b/Tests/UnitTests/Plugins/DD4hep/DD4hepCylindricalDetectorTests.cpp @@ -160,10 +160,10 @@ Acts::Test::CylindricalTrackingGeometry::DetectorStore generateXML() { std::vector> innerOuter = { {25., 35.}, {65., 75.}, {110., 120.}}; auto b0Surfaces = cGeometry.surfacesCylinder(dStore, 8.4, 36., 0.15, 0.14, - 32., 3., 2., {16, 14}); + 31., 3., 2., {16, 14}); auto b1Surfaces = cGeometry.surfacesCylinder(dStore, 8.4, 36., 0.15, 0.14, - 72., 3., 2., {32, 14}); + 71., 3., 2., {32, 14}); auto b2Surfaces = cGeometry.surfacesCylinder(dStore, 8.4, 36., 0.15, 0.14, 116., 3., 2., {52, 14}); @@ -192,7 +192,7 @@ Acts::Test::CylindricalTrackingGeometry::DetectorStore generateXML() { cxml << beampipe_head_xml << '\n'; cxml << indent_12_xml << "" << '\n'; cxml << indent_12_xml - << "" << '\n'; cxml << indent_12_xml << "" << '\n'; diff --git a/Tests/UnitTests/Plugins/Json/DetectorJsonConverterTests.cpp b/Tests/UnitTests/Plugins/Json/DetectorJsonConverterTests.cpp index b4508076a2c..39ede9a27f8 100644 --- a/Tests/UnitTests/Plugins/Json/DetectorJsonConverterTests.cpp +++ b/Tests/UnitTests/Plugins/Json/DetectorJsonConverterTests.cpp @@ -178,7 +178,7 @@ BOOST_AUTO_TEST_CASE(BeamPipeEndcapBarrelDetector) { Acts::Logging::VERBOSE)); Acts::Experimental::VolumeStructureBuilder::Config shapeConfig; - shapeConfig.boundValues = {20, 100, 10., M_PI, 0.}; + shapeConfig.boundValues = {18, 100, 10., M_PI, 0.}; shapeConfig.transform = Acts::Transform3(Acts::Transform3::Identity()) .pretranslate(Acts::Vector3(0., 0., ep)); shapeConfig.boundsType = Acts::VolumeBounds::BoundsType::eCylinder; @@ -202,7 +202,7 @@ BOOST_AUTO_TEST_CASE(BeamPipeEndcapBarrelDetector) { // Central barrel Acts::Experimental::VolumeStructureBuilder::Config innerShapeConfig; - innerShapeConfig.boundValues = {20., 60., 700., M_PI, 0.}; + innerShapeConfig.boundValues = {18., 60., 700., M_PI, 0.}; innerShapeConfig.boundsType = Acts::VolumeBounds::BoundsType::eCylinder; auto innerShapeBuilder = @@ -298,7 +298,7 @@ BOOST_AUTO_TEST_CASE(BeamPipeEndcapBarrelDetector) { // Beam Pipe Acts::Experimental::VolumeStructureBuilder::Config bpShapeConfig; - bpShapeConfig.boundValues = {0., 20., 720., M_PI, 0.}; + bpShapeConfig.boundValues = {0., 18., 720., M_PI, 0.}; bpShapeConfig.boundsType = Acts::VolumeBounds::BoundsType::eCylinder; auto bpShapeBuilder = diff --git a/Tests/UnitTests/Plugins/Json/DetectorVolumeJsonConverterTests.cpp b/Tests/UnitTests/Plugins/Json/DetectorVolumeJsonConverterTests.cpp index 2cf710fc7e8..27ed95d7bf2 100644 --- a/Tests/UnitTests/Plugins/Json/DetectorVolumeJsonConverterTests.cpp +++ b/Tests/UnitTests/Plugins/Json/DetectorVolumeJsonConverterTests.cpp @@ -14,6 +14,7 @@ #include "Acts/Detector/LayerStructureBuilder.hpp" #include "Acts/Detector/PortalGenerators.hpp" #include "Acts/Detector/VolumeStructureBuilder.hpp" +#include "Acts/Geometry/ConeVolumeBounds.hpp" #include "Acts/Geometry/CylinderVolumeBounds.hpp" #include "Acts/Geometry/GeometryContext.hpp" #include "Acts/Navigation/DetectorVolumeFinders.hpp"