From 5a420db153a5e3d55274ed021c089b720bf403bb Mon Sep 17 00:00:00 2001 From: Berggren-Jonas <93843498+Berggren-Jonas@users.noreply.github.com> Date: Mon, 16 Sep 2024 09:58:17 +0200 Subject: [PATCH] feat: generalized conversion of FullPhysicalVolume (#3585) This PR generalizes the `GeoModelDetectorObjectFactory` by enabling the conversion of `GeoFullPhysicalVolume` to `DetectorVolume` in the case where they don't have any subvolumes. This feature is not required for ATLAS application as far as I know, but is still nice to have for generality's sake and makes the testing an prototyping of geometries more intuitive. --- .../src/GeoModelDetectorObjectFactory.cpp | 53 +++++++++---------- .../Plugins/GeoModel/GeoBoxToVolumeTest.cpp | 7 --- .../GeoModel/GeoDetectorObjectTest.cpp | 2 - .../Plugins/GeoModel/GeoTrdToVolumeTests.cpp | 8 --- .../Plugins/GeoModel/GeoTubeToVolumeTest.cpp | 15 ++---- 5 files changed, 30 insertions(+), 55 deletions(-) diff --git a/Plugins/GeoModel/src/GeoModelDetectorObjectFactory.cpp b/Plugins/GeoModel/src/GeoModelDetectorObjectFactory.cpp index d6def3bd66c..3952d93387a 100644 --- a/Plugins/GeoModel/src/GeoModelDetectorObjectFactory.cpp +++ b/Plugins/GeoModel/src/GeoModelDetectorObjectFactory.cpp @@ -155,38 +155,37 @@ void Acts::GeoModelDetectorObjectFactory::convertFpv( // get children std::vector subvolumes = getChildrenWithRef(physVol, false); - if (!subvolumes.empty()) { - // vector containing all subvolumes to be converted to surfaces - std::vector surfaces = findAllSubVolumes(physVol); - std::vector sensitives; - - for (const auto &surface : surfaces) { - const Transform3 &transform = - fpv->getAbsoluteTransform() * surface.transform; - convertSensitive(surface.volume, transform, sensitives); - } - cache.sensitiveSurfaces.insert(cache.sensitiveSurfaces.end(), - sensitives.begin(), sensitives.end()); - // TODO maybe put that down after an surface conversions - if (convertBox(name)) { - const GeoLogVol *logVol = - physVol->getLogVol(); // get logVol for the shape of the volume - const GeoShape *shape = logVol->getShape(); // get shape - const Acts::Transform3 &fpvtransform = fpv->getAbsoluteTransform(nullptr); - - // convert bounding boxes with surfaces inside - std::shared_ptr box = - Acts::GeoModel::convertDetectorVolume(gctx, *shape, name, - fpvtransform, sensitives); - cache.boundingBoxes.push_back(box); - } - } else { + // vector containing all subvolumes to be converted to surfaces + std::vector surfaces = findAllSubVolumes(physVol); + std::vector sensitives; + + for (const auto &surface : surfaces) { + const Transform3 &transform = + fpv->getAbsoluteTransform() * surface.transform; + convertSensitive(surface.volume, transform, sensitives); + } + cache.sensitiveSurfaces.insert(cache.sensitiveSurfaces.end(), + sensitives.begin(), sensitives.end()); + if (convertBox(name)) { + const GeoLogVol *logVol = + physVol->getLogVol(); // get logVol for the shape of the volume + const GeoShape *shape = logVol->getShape(); // get shape + const Acts::Transform3 &fpvtransform = fpv->getAbsoluteTransform(nullptr); + + // convert bounding boxes with surfaces inside + std::shared_ptr box = + Acts::GeoModel::convertDetectorVolume(gctx, *shape, name, fpvtransform, + sensitives); + cache.boundingBoxes.push_back(box); + } + // If fpv has no subs and should not be converted to volume convert to surface + else if (subvolumes.empty()) { // convert fpvs to surfaces const Transform3 &transform = fpv->getAbsoluteTransform(); convertSensitive(fpv, transform, cache.sensitiveSurfaces); } } -// lambda to determine if object fits query +// function to determine if object fits query bool Acts::GeoModelDetectorObjectFactory::matches(const std::string &name, const PVConstLink &physvol) { if (m_cfg.nameList.empty() && m_cfg.materialList.empty()) { diff --git a/Tests/UnitTests/Plugins/GeoModel/GeoBoxToVolumeTest.cpp b/Tests/UnitTests/Plugins/GeoModel/GeoBoxToVolumeTest.cpp index cf7106b265e..035fe7ebc1a 100644 --- a/Tests/UnitTests/Plugins/GeoModel/GeoBoxToVolumeTest.cpp +++ b/Tests/UnitTests/Plugins/GeoModel/GeoBoxToVolumeTest.cpp @@ -43,13 +43,6 @@ BOOST_AUTO_TEST_CASE(GeoBoxToSensitiveConversion) { auto logBox = new GeoLogVol("Box", box, material); auto physBox = make_intrusive(logBox); - // add subvolume since converter needs that to convert fpv to volume - auto sBox = new GeoBox(50, 20, 10); - auto slogBox = new GeoLogVol("Box", sBox, material); - auto sphysBox = make_intrusive(slogBox); - - physBox->add(sphysBox); - // create pars for conversion Acts::GeoModelDetectorObjectFactory::Config gmConfig; gmConfig.convertBox = {"Box"}; diff --git a/Tests/UnitTests/Plugins/GeoModel/GeoDetectorObjectTest.cpp b/Tests/UnitTests/Plugins/GeoModel/GeoDetectorObjectTest.cpp index 856ffb361e5..ae155533c30 100644 --- a/Tests/UnitTests/Plugins/GeoModel/GeoDetectorObjectTest.cpp +++ b/Tests/UnitTests/Plugins/GeoModel/GeoDetectorObjectTest.cpp @@ -38,7 +38,6 @@ struct GeoDims { std::vector> trapVerts; std::vector trapHls; std::vector> polyVerts; - // double poly_z; }; void test(const Acts::GeoModelDetectorObjectFactory::Cache& cache, GeoModelDetObj::GeoDims geoDims) { @@ -101,7 +100,6 @@ GeoGeometry constructGeoModel() { geoDims.polyVerts = {{-60, -50}, {60, -50}, {153, 0}, {123, 50}, {-123, 50}, {-153, 0}}; geoDims.tube = {5, 6, 100}; - // geoDims.poly_z = 2; geoDims.trapHls = { fabs(geoDims.trapVerts[0][0] - geoDims.trapVerts[1][0]) / 2, fabs(geoDims.trapVerts[2][0] - geoDims.trapVerts[3][0]) / 2, diff --git a/Tests/UnitTests/Plugins/GeoModel/GeoTrdToVolumeTests.cpp b/Tests/UnitTests/Plugins/GeoModel/GeoTrdToVolumeTests.cpp index 8b1daea6c26..6f8794bce0e 100644 --- a/Tests/UnitTests/Plugins/GeoModel/GeoTrdToVolumeTests.cpp +++ b/Tests/UnitTests/Plugins/GeoModel/GeoTrdToVolumeTests.cpp @@ -37,19 +37,11 @@ BOOST_AUTO_TEST_CASE(GeoTrdToVolumeConversion) { auto logTrd = new GeoLogVol("Trd", trd, material); auto physTrd = make_intrusive(logTrd); - // add subvolume since converter needs that to convert fpv to volume - auto strd = new GeoTrd(1, 1, 25, 40, 30); - auto slogTrd = new GeoLogVol("Trd", strd, material); - auto sphysTrd = make_intrusive(slogTrd); - // this should produce an error while converting auto errTrd = new GeoTrd(2, 3, 25, 40, 30); auto errLogTrd = new GeoLogVol("Trd", errTrd, material); auto errPhysTrd = make_intrusive(errLogTrd); - physTrd->add(sphysTrd); - errPhysTrd->add(sphysTrd); - // create pars for conversion Acts::GeoModelDetectorObjectFactory::Config gmConfig; gmConfig.convertBox = {"Trd"}; diff --git a/Tests/UnitTests/Plugins/GeoModel/GeoTubeToVolumeTest.cpp b/Tests/UnitTests/Plugins/GeoModel/GeoTubeToVolumeTest.cpp index cb8fde09f1b..c6cd459b35a 100644 --- a/Tests/UnitTests/Plugins/GeoModel/GeoTubeToVolumeTest.cpp +++ b/Tests/UnitTests/Plugins/GeoModel/GeoTubeToVolumeTest.cpp @@ -8,11 +8,11 @@ #include -// switching format off to avoid conflicting declaration in GeoModel -// needed until Acts GeoModel bumps to 6.5 -//clang-format off +// In order to avoid conflicts with declarations in Geomodel that is fixed in +// v3.5 +// clang-formal off #include "Acts/Plugins/GeoModel/GeoModelDetectorObjectFactory.hpp" -//clang-format on +// clang-formal on #include "Acts/Geometry/CylinderVolumeBounds.hpp" #include "Acts/Geometry/GeometryContext.hpp" #include "Acts/Plugins/GeoModel/GeoModelConverters.hpp" @@ -41,13 +41,6 @@ BOOST_AUTO_TEST_CASE(GeoBoxToSensitiveConversion) { auto logTube = new GeoLogVol("Tube", tube, material); auto physTube = make_intrusive(logTube); - // add subvolume since converter needs that to convert fpv to volume - auto sTube = new GeoTube(50, 20, 10); - auto slogTube = new GeoLogVol("Tube", sTube, material); - auto sphysTube = make_intrusive(slogTube); - - physTube->add(sphysTube); - // create pars for conversion Acts::GeoModelDetectorObjectFactory::Config gmConfig; gmConfig.convertBox = {"Tube"};