From c3eea1c2123b1cb7f72276d909ad0d4224a74a03 Mon Sep 17 00:00:00 2001 From: ssdetlab <113530373+ssdetlab@users.noreply.github.com> Date: Wed, 20 Mar 2024 11:35:59 +0200 Subject: [PATCH] feat: Geant4SurfaceProvider optimization (#3025) Introducing minor changes to the `Geant4SurfaceProvider` targeting optimization and utility for Telescope-style use cases. The first change is the removal of the Gdml parsing from the construction in favor of supplying a pointer to the world volume. The Internal Structure Builder is called (potentially) multiple times in the builder pipeline, so, before the changes, the description was also parsed multiple times and several Worlds were kept in the memory. The second change has to do with the conventional z-orientation of the Telescope geometries. There's a need to rotate the geometries away from the z-axis in the current implementation of the track parametrization, so a simple way of applying a global transformation at the construction stage is introduced. In our experience, the cleanest way to do it is on the Geant4 volumes conversion stage, thus the additional `G4Transform` is included into the `Config`. --- .../Plugins/Geant4/Geant4SurfaceProvider.hpp | 35 +++++++--------- .../Geant4/Geant4SurfaceProviderTests.cpp | 40 +++++++++++++------ 2 files changed, 42 insertions(+), 33 deletions(-) diff --git a/Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4SurfaceProvider.hpp b/Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4SurfaceProvider.hpp index bbd4828e11d..f7c82647042 100644 --- a/Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4SurfaceProvider.hpp +++ b/Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4SurfaceProvider.hpp @@ -43,8 +43,8 @@ class Geant4SurfaceProvider : public Acts::Experimental::ISurfacesProvider { public: /// Nested configuration struct struct Config { - /// The path of the gdml file - std::string gdmlPath = ""; + /// Pointer to the g4World volume + const G4VPhysicalVolume* g4World = nullptr; /// Convert the length scale ActsScalar scaleConversion = 1.; @@ -55,6 +55,10 @@ class Geant4SurfaceProvider : public Acts::Experimental::ISurfacesProvider { /// Converted material thickness (< 0 indicates keeping original thickness) ActsScalar convertedMaterialThickness = -1; + /// Transformation to apply to the + /// G4World volume + G4Transform3D worldTransform = G4Transform3D(); + /// A selector for passive surfaces std::shared_ptr surfacePreselector = std::make_shared(); @@ -85,13 +89,11 @@ class Geant4SurfaceProvider : public Acts::Experimental::ISurfacesProvider { /// Constructor /// @param config The configuration struct /// @param options The optional configuration for KDTree - /// @param validateGDMLschema Whether to validate the gdml schema Geant4SurfaceProvider(const Config& config, - const kdtOptions& options = kdtOptions(), - bool validateGDMLschema = true) { - if (config.gdmlPath.empty()) { + const kdtOptions& options = kdtOptions()) { + if (config.g4World == nullptr) { throw std::invalid_argument( - "Geant4SurfaceProvider: no gdml file provided"); + "Geant4SurfaceProvider: No World volume provided"); } if (config.surfacePreselector == nullptr) { throw std::invalid_argument( @@ -100,16 +102,8 @@ class Geant4SurfaceProvider : public Acts::Experimental::ISurfacesProvider { m_cfg = config; m_kdtOptions = options; - - /// Read the gdml file and get the world volume - G4GDMLParser parser; - parser.Read(m_cfg.gdmlPath, validateGDMLschema); - m_g4World = parser.GetWorldVolume(); - - if (m_g4World == nullptr) { - throw std::invalid_argument( - "Geant4SurfaceProvider: No g4World initialized"); - } + m_g4World = m_cfg.g4World; + m_g4ToWorld = m_cfg.worldTransform; }; /// Destructor @@ -131,11 +125,10 @@ class Geant4SurfaceProvider : public Acts::Experimental::ISurfacesProvider { /// Generate the surface cache Acts::Geant4DetectorSurfaceFactory::Cache g4SurfaceCache; - G4Transform3D g4ToWorld; /// Find and store surfaces in the cache object Acts::Geant4DetectorSurfaceFactory{}.construct( - g4SurfaceCache, g4ToWorld, *m_g4World, g4SurfaceOptions); + g4SurfaceCache, m_g4ToWorld, *m_g4World, g4SurfaceOptions); auto surfaces = g4SurfaceCache.passiveSurfaces; @@ -157,7 +150,9 @@ class Geant4SurfaceProvider : public Acts::Experimental::ISurfacesProvider { kdtOptions m_kdtOptions; - G4VPhysicalVolume* m_g4World = nullptr; + const G4VPhysicalVolume* m_g4World; + + G4Transform3D m_g4ToWorld; }; } // namespace Experimental diff --git a/Tests/UnitTests/Plugins/Geant4/Geant4SurfaceProviderTests.cpp b/Tests/UnitTests/Plugins/Geant4/Geant4SurfaceProviderTests.cpp index 1d4f45cbc70..d5de645fc17 100644 --- a/Tests/UnitTests/Plugins/Geant4/Geant4SurfaceProviderTests.cpp +++ b/Tests/UnitTests/Plugins/Geant4/Geant4SurfaceProviderTests.cpp @@ -113,17 +113,21 @@ auto gctx = Acts::GeometryContext(); auto [physWorld, names] = ConstructGeant4World(); BOOST_AUTO_TEST_CASE(Geant4SurfaceProviderNames) { + /// Read the gdml file and get the world volume + G4GDMLParser parser; + parser.Read(gdmlPath.string(), false); + auto world = parser.GetWorldVolume(); + // Default template parameters are fine // when using names as identifiers auto spFullCfg = Acts::Experimental::Geant4SurfaceProvider<>::Config(); - spFullCfg.gdmlPath = gdmlPath.string(); + spFullCfg.g4World = world; spFullCfg.surfacePreselector = std::make_shared(names, true); auto spFull = std::make_shared>( - spFullCfg, Acts::Experimental::Geant4SurfaceProvider<>::kdtOptions(), - false); + spFullCfg, Acts::Experimental::Geant4SurfaceProvider<>::kdtOptions()); auto lbFullCfg = Acts::Experimental::LayerStructureBuilder::Config(); lbFullCfg.surfacesProvider = spFull; @@ -157,7 +161,7 @@ BOOST_AUTO_TEST_CASE(Geant4SurfaceProviderNames) { names.begin() + names.size() / 2); auto spLeftArmCfg = Acts::Experimental::Geant4SurfaceProvider<>::Config(); - spLeftArmCfg.gdmlPath = gdmlPath.string(); + spLeftArmCfg.g4World = world; spLeftArmCfg.surfacePreselector = std::make_shared( leftArmNames, true); @@ -165,7 +169,7 @@ BOOST_AUTO_TEST_CASE(Geant4SurfaceProviderNames) { auto spLeftArm = std::make_shared>( spLeftArmCfg, - Acts::Experimental::Geant4SurfaceProvider<>::kdtOptions(), false); + Acts::Experimental::Geant4SurfaceProvider<>::kdtOptions()); auto lbCfg = Acts::Experimental::LayerStructureBuilder::Config(); lbCfg.surfacesProvider = spLeftArm; @@ -190,9 +194,14 @@ BOOST_AUTO_TEST_CASE(Geant4SurfaceProviderNames) { } BOOST_AUTO_TEST_CASE(Geant4SurfaceProviderRanges) { + /// Read the gdml file and get the world volume + G4GDMLParser parser; + parser.Read(gdmlPath.string(), false); + auto world = parser.GetWorldVolume(); + // 1D selection -- select only the second row auto sp1DCfg = Acts::Experimental::Geant4SurfaceProvider<1>::Config(); - sp1DCfg.gdmlPath = gdmlPath.string(); + sp1DCfg.g4World = world; auto kdt1DOpt = Acts::Experimental::Geant4SurfaceProvider<1>::kdtOptions(); kdt1DOpt.range = Acts::RangeXD<1, Acts::ActsScalar>(); @@ -200,7 +209,7 @@ BOOST_AUTO_TEST_CASE(Geant4SurfaceProviderRanges) { kdt1DOpt.binningValues = {Acts::BinningValue::binZ}; auto sp1D = std::make_shared>( - sp1DCfg, kdt1DOpt, false); + sp1DCfg, kdt1DOpt); auto lb1DCfg = Acts::Experimental::LayerStructureBuilder::Config(); lb1DCfg.surfacesProvider = sp1D; @@ -226,7 +235,7 @@ BOOST_AUTO_TEST_CASE(Geant4SurfaceProviderRanges) { // 2D selection -- select only the second row // of the left arm auto sp2DCfg = Acts::Experimental::Geant4SurfaceProvider<2>::Config(); - sp2DCfg.gdmlPath = gdmlPath.string(); + sp2DCfg.g4World = world; auto kdt2DOpt = Acts::Experimental::Geant4SurfaceProvider<2>::kdtOptions(); kdt2DOpt.range = Acts::RangeXD<2, Acts::ActsScalar>(); @@ -235,7 +244,7 @@ BOOST_AUTO_TEST_CASE(Geant4SurfaceProviderRanges) { kdt2DOpt.binningValues = {Acts::BinningValue::binZ}; auto sp2D = std::make_shared>( - sp2DCfg, kdt2DOpt, false); + sp2DCfg, kdt2DOpt); auto lb2DCfg = Acts::Experimental::LayerStructureBuilder::Config(); lb2DCfg.surfacesProvider = sp2D; @@ -257,7 +266,7 @@ BOOST_AUTO_TEST_CASE(Geant4SurfaceProviderRanges) { // Preselect the left arm based on the position // and select only the second row auto sp2DPosCfg = Acts::Experimental::Geant4SurfaceProvider<1>::Config(); - sp2DPosCfg.gdmlPath = gdmlPath.string(); + sp2DPosCfg.g4World = world; std::map> ranges; std::array g4Axes{0}; @@ -274,7 +283,7 @@ BOOST_AUTO_TEST_CASE(Geant4SurfaceProviderRanges) { ranges); auto sp2DPos = std::make_shared>( - sp2DPosCfg, kdt1DOpt, false); + sp2DPosCfg, kdt1DOpt); auto lb2DPosCfg = Acts::Experimental::LayerStructureBuilder::Config(); lb2DPosCfg.surfacesProvider = sp2DPos; @@ -345,10 +354,15 @@ BOOST_AUTO_TEST_CASE(Geant4RectangleFromGDML) { bgdml.close(); + /// Read the gdml file and get the world volume + G4GDMLParser parser; + parser.Read("Plane.gdml", false); + auto world = parser.GetWorldVolume(); + // 1D selection -- select only the second row auto planeFromGDMLCfg = Acts::Experimental::Geant4SurfaceProvider<1>::Config(); - planeFromGDMLCfg.gdmlPath = "Plane.gdml"; + planeFromGDMLCfg.g4World = world; planeFromGDMLCfg.surfacePreselector = std::make_shared( std::vector{"b_pv"}, true); @@ -362,7 +376,7 @@ BOOST_AUTO_TEST_CASE(Geant4RectangleFromGDML) { auto planeProvider = std::make_shared>( - planeFromGDMLCfg, kdt1DOpt, false); + planeFromGDMLCfg, kdt1DOpt); auto planes = planeProvider->surfaces(tContext); BOOST_CHECK_EQUAL(planes.size(), 1u);