MuonR4 geometry - Fix crashes when running with multiple threads

MuonR4 geometry - Fix crashes when running with multiple threads

DetectorDescription/GeoModel/GeoModelUtilities/GeoModelUtilities/GeoAlignmentStore.h

@@ -15,6 +15,9 @@
 
 #include "GeoModelKernel/GeoDefinitions.h"
 #include "GeoModelKernel/GeoVAlignmentStore.h"
+#include "GeoModelKernel/GeoVFullPhysVol.h"
+#include "GeoModelKernel/GeoAlignableTransform.h"
+
 #include "GeoModelUtilities/TransformMap.h"
 #include "GeoModelUtilities/TransientConstSharedPtr.h"
 
@@ -86,16 +89,13 @@ public:
     PositioningMapPtr getDefAbsPositions() const;
 };
 
-ATH_ALWAYS_INLINE
-const GeoTrf::Transform3D* GeoAlignmentStore::getDelta(const GeoAlignableTransform* axf) const { 
+inline const GeoTrf::Transform3D* GeoAlignmentStore::getDelta(const GeoAlignableTransform* axf) const { 
     return m_deltas->getTransform(axf); 
 }
-ATH_ALWAYS_INLINE
-const GeoTrf::Transform3D* GeoAlignmentStore::getAbsPosition(const GeoVFullPhysVol* fpv) const {
+inline const GeoTrf::Transform3D* GeoAlignmentStore::getAbsPosition(const GeoVFullPhysVol* fpv) const {
     return m_absPositions->getTransform(fpv); 
 }
-ATH_ALWAYS_INLINE
-const GeoTrf::Transform3D* GeoAlignmentStore::getDefAbsPosition(const GeoVFullPhysVol* fpv) const {
+inline const GeoTrf::Transform3D* GeoAlignmentStore::getDefAbsPosition(const GeoVFullPhysVol* fpv) const {
     return m_defAbsPositions->getTransform(fpv);
 }
 

DetectorDescription/GeoModel/GeoModelUtilities/GeoModelUtilities/TransientConstSharedPtr.h

@@ -81,6 +81,8 @@ namespace GeoModel {
         /// Is the pointer defined
         operator bool() const { return m_ptr.get() != nullptr; }
         bool operator!() const { return !m_ptr; }
+        /// How many clients does the pointer have
+        size_t use_count() const { return m_ptr.use_count(); }
 
         /// Smaller operator to insert the pointer into sets
         bool operator<(const TransientConstSharedPtr& other) const { return m_ptr.get() < other.m_ptr.get(); }

MuonSpectrometer/MuonPhaseII/MuonConditions/MuonCondAlgR4/python/ConditionsConfig.py

@@ -11,6 +11,9 @@ def ActsMuonAlignCondAlgCfg(flags, name="ActsMuonAlignCondAlg", **kwargs):
     
     from MuonConfig.MuonGeometryConfig import MuonAlignmentCondAlgCfg
     kwargs.setdefault("applyMmPassivation", flags.Muon.applyMMPassivation)
+    kwargs.setdefault("FillAlignCache", True)
+    kwargs.setdefault("FillGeoAlignStore", False)
+    
 
     if kwargs["applyMmPassivation"]:
         from MuonConfig.MuonCondAlgConfig import NswPassivationDbAlgCfg

MuonSpectrometer/MuonPhaseII/MuonConditions/MuonCondAlgR4/src/ActsMuonAlignCondAlg.cxx

@@ -275,7 +275,19 @@ StatusCode ActsMuonAlignCondAlg::execute(const EventContext& ctx) const {
         }
         /// Propagate the cache throughout the geometry
         ATH_CHECK(declareDependencies(ctx, subDet, writeHandle));
-        if (m_fillAlignStoreCache) {
+        if (m_fillGeoAlignStore) {
+            /// Ensure that the rigid transformations of the detector elements are applied 
+            for (const MuonReadoutElement* re : readoutEles) {
+                if (re->detectorType() == subDet) {
+                    const Amg::Transform3D& detTrf{re->getMaterialGeom()->getAbsoluteTransform(writeCdo->geoModelAlignment.get())};
+                    ATH_MSG_VERBOSE("Detector element "<<m_idHelperSvc->toStringDetEl(re->identify())<<" is located at "
+                                    <<Amg::toString(detTrf));
+                }
+            }
+            /// There's no need to cache the delta parameters longer
+            writeCdo->geoModelAlignment->getDeltas()->clear();
+            writeCdo->geoModelAlignment->lockPosCache();
+        } else if (m_fillAlignStoreCache) {
             for (const MuonReadoutElement* re : readoutEles){
                 numAligned+= re->storeAlignedTransforms(*writeCdo);
             }
@@ -284,11 +296,8 @@ StatusCode ActsMuonAlignCondAlg::execute(const EventContext& ctx) const {
         }
         ATH_CHECK(writeHandle.record(std::move(writeCdo)));
     }
-    /// Check that all readout elements were properly aligned
-    if (m_fillAlignStoreCache && numAligned != readoutEles.size()){
-        ATH_MSG_FATAL("Only "<<numAligned<<" out of "<<readoutEles.size()<<" were picked up by the alignment cutalg");
-        return StatusCode::FAILURE;
-    }
+
+    ATH_MSG_VERBOSE("Only "<<numAligned<<" out of "<<readoutEles.size()<<" were picked up by the alignment cutalg");
     alignDeltas.clear();
     techTransforms.clear();
     /// Whipe the GeoModelCache

MuonSpectrometer/MuonPhaseII/MuonConditions/MuonCondAlgR4/src/ActsMuonAlignCondAlg.h

@@ -99,6 +99,8 @@ private:
     Gaudi::Property<bool> m_applyBLines{this, "applyBLines", false};
     /// Flag toggling whether the alignment store shall be filled with the transforms or not
     Gaudi::Property<bool> m_fillAlignStoreCache{this, "FillAlignCache", false};
+    /// Flag toggling whether the GeoAlignmentStore shall be filled
+    Gaudi::Property<bool> m_fillGeoAlignStore{this, "FillGeoAlignStore", true};
 
 };
 

MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonGeoModelR4/python/MuonGeoModelConfig.py

@@ -95,7 +95,8 @@ def MuonAlignStoreCfg(flags):
                                                   name="ActsDetAlignmentAlgMdt",
                                                   CondAlignStore="MdtActsAlignContainer" if flags.Muon.enableAlignment else "",
                                                   EventAlignStore="MdtActsAlignContainer",
-                                                  SplitPhysVolCache = True,
+                                                  SplitPhysVolCache = False,
+                                                  SplitActsTrfCache = False,
                                                   FillAlignCache = False,
                                                   LoadTrackingGeoSvc = False,
                                                   DetectorType=DetectorType.Mdt))
@@ -104,7 +105,8 @@ def MuonAlignStoreCfg(flags):
                                                   name="ActsDetAlignmentAlgRpc",
                                                   CondAlignStore="RpcActsAlignContainer" if flags.Muon.enableAlignment else "",
                                                   EventAlignStore="RpcActsAlignContainer",
-                                                  SplitPhysVolCache = True,
+                                                  SplitPhysVolCache = False,
+                                                  SplitActsTrfCache = False,
                                                   FillAlignCache = False,
                                                   LoadTrackingGeoSvc = False,
                                                   DetectorType=DetectorType.Rpc))
@@ -113,7 +115,8 @@ def MuonAlignStoreCfg(flags):
                                                   name="ActsDetAlignmentAlgTgc",
                                                   CondAlignStore="TgcActsAlignContainer" if flags.Muon.enableAlignment else "",
                                                   EventAlignStore="TgcActsAlignContainer",
-                                                  SplitPhysVolCache = True,
+                                                  SplitPhysVolCache = False,
+                                                  SplitActsTrfCache = False,
                                                   FillAlignCache = False,
                                                   LoadTrackingGeoSvc = False,
                                                   DetectorType=DetectorType.Tgc))
@@ -123,6 +126,7 @@ def MuonAlignStoreCfg(flags):
                                                   CondAlignStore="sTgcActsAlignContainer" if flags.Muon.enableAlignment else "",
                                                   EventAlignStore="sTgcActsAlignContainer",
                                                   SplitPhysVolCache = False,
+                                                  SplitActsTrfCache = False,
                                                   FillAlignCache = False,
                                                   LoadTrackingGeoSvc = False,
                                                   DetectorType=DetectorType.sTgc))
@@ -134,6 +138,7 @@ def MuonAlignStoreCfg(flags):
                                                                                            flags.Muon.applyMMPassivation else "",
                                                   EventAlignStore="MmActsAlignContainer",
                                                   SplitPhysVolCache = False,
+                                                  SplitActsTrfCache = False,
                                                   FillAlignCache = False,
                                                   LoadTrackingGeoSvc = False,
                                                   DetectorType=DetectorType.Mm))

MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/MuonReadoutGeometryR4/MdtReadoutElement.icc

 /*
-  Copyright (C) 2002-2023 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 #ifndef MUONREADOUTGEOMETRYR4_MDTREADOUTELEMENT_ICC
 #define MUONREADOUTGEOMETRYR4_MDTREADOUTELEMENT_ICC
@@ -8,8 +8,11 @@
 namespace ActsTrk{
     template <> inline Amg::Transform3D 
         TransformCacheDetEle<MuonGMR4::MdtReadoutElement>::fetchTransform(const DetectorAlignStore* store) const {
-            const unsigned int tubeNum = m_parent->tubeNumber(hash()) + 1;
-            if (tubeNum == 0) {
+            using RE = MuonGMR4::MdtReadoutElement;
+            static const unsigned int layerTube = RE::tubeNumber(RE::measurementHash(0,0));
+            /// Check whether the hash represents a tube or the central layer
+            const unsigned int tubeNum = m_parent->tubeNumber(hash());
+            if (tubeNum == layerTube) {
                 const Amg::Translation3D toCenter{0.5*m_parent->moduleHeight() * Amg::Vector3D::UnitY()};
                 return m_parent->toStation(store) * m_parent->toChamberLayer(hash()) *
                         toCenter * Amg::getRotateY3D(90*Gaudi::Units::deg); 

MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/MuonReadoutGeometryR4/MuonReadoutElement.h

@@ -168,7 +168,7 @@ class MuonReadoutElement : public GeoVDetectorElement, public AthMessaging, publ
      friend class ActsTrk::TransformCacheDetEle<MuonGMR4::MuonReadoutElement>;
    protected:
      /// Returns the transformation into the center of the readout volume
-     Amg::Transform3D toStation(const ActsTrk::DetectorAlignStore* alignStore) const;
+     const Amg::Transform3D& toStation(const ActsTrk::DetectorAlignStore* alignStore) const;
       
      /// Inserts a transfomration for caching
      template <class MuonDetImpl> StatusCode insertTransform(const IdentifierHash& hash);

MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/src/MuonReadoutElement.cxx

@@ -51,7 +51,7 @@ const Amg::Transform3D& MuonReadoutElement::localToGlobalTrans(const ActsGeometr
     return dummyTrans;
 }
 
-Amg::Transform3D MuonReadoutElement::toStation(const DetectorAlignStore* alignStore) const {
+const Amg::Transform3D& MuonReadoutElement::toStation(const DetectorAlignStore* alignStore) const {
    return getMaterialGeom()->getAbsoluteTransform(alignStore ? alignStore->geoModelAlignment.get() : nullptr);
 }
 void MuonReadoutElement::releaseUnAlignedTrfs() const {

Tracking/Acts/ActsAlignmentAlgs/src/AlignStoreProviderAlg.cxx

@@ -56,7 +56,12 @@ StatusCode AlignStoreProviderAlg::execute(const EventContext& ctx) const {
         newAlignment = std::make_unique<DetectorAlignStore>(**readHandle);
         /// Setup a separate cache for the full physical volume transfomrations
         if (m_splitPhysVolCache) {
-            if (newAlignment->geoModelAlignment) newAlignment->geoModelAlignment->clearPosCache();
+            if (newAlignment->geoModelAlignment) {
+                newAlignment->geoModelAlignment = std::make_unique<GeoAlignmentStore>(*newAlignment->geoModelAlignment);
+                newAlignment->geoModelAlignment->clearPosCache();
+            }
+        }
+        if (m_splitActsTrfCache) {
             using TrackingStore = DetectorAlignStore::TrackingAlignStore;
             newAlignment->trackingAlignment = std::make_unique<TrackingStore>(newAlignment->detType);
         }

Tracking/Acts/ActsAlignmentAlgs/src/AlignStoreProviderAlg.h

@@ -44,6 +44,8 @@ namespace ActsTrk{
       /// Flag toggling whether the full GeoAlignmentStore shall be written to store gate or whether the
       /// absolute transforms are split into two different stores
       Gaudi::Property<bool> m_splitPhysVolCache{this, "SplitPhysVolCache", false};
+      /// Flag toggline whether the final transforms of the ActsDet volumes shall be split per event
+      Gaudi::Property<bool> m_splitActsTrfCache{this, "SplitActsTrfCache", false};
       /// Flag toggling whether the alignment store shall be filled with the transforms or not
       Gaudi::Property<bool> m_fillAlignStoreCache{this, "FillAlignCache", true};
       /// Static cast of >DetectorType< property

Tracking/Acts/ActsGeoUtils/CMakeLists.txt

@@ -18,18 +18,19 @@ atlas_add_library( ActsGeoUtils
                    LINK_LIBRARIES ${GEOMODEL_LIBRARIES} GeoPrimitives Identifier 
                                   ActsGeometryInterfacesLib GeoModelUtilities CxxUtils)
 
-# we also add unit tests. 
-# Build them from the files in test/ 
-file(GLOB_RECURSE tests "test/*.cxx")
-
-foreach(_theTestSource ${tests})
-    get_filename_component(_theTest ${_theTestSource} NAME_WE)
-    atlas_add_test( ${_theTest} SOURCES ${_theTestSource}
-                    INCLUDE_DIRS ${ROOT_INCLUDE_DIRS} 
-                    LINK_LIBRARIES ${ROOT_LIBRARIES} ActsGeoUtils 
-                    POST_EXEC_SCRIPT nopost.sh)
-    
+if( NOT SIMULATIONBASE )                                                            
+    # we also add unit tests. 
+    # Build them from the files in test/ 
+    file(GLOB_RECURSE tests "test/*.cxx")
+
+    foreach(_theTestSource ${tests})
+        get_filename_component(_theTest ${_theTestSource} NAME_WE)
+        atlas_add_test( ${_theTest} SOURCES ${_theTestSource}
+                        INCLUDE_DIRS ${ROOT_INCLUDE_DIRS} 
+                        LINK_LIBRARIES ${ROOT_LIBRARIES} ActsGeoUtils 
+                        POST_EXEC_SCRIPT nopost.sh)
+    endforeach()
+endif()
 
-endforeach()
     
 

Tracking/Acts/ActsGeoUtils/test/test_AlignStoreAccess.cxx

+/*
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+*/
+
+#include "GeoPrimitives/GeoPrimitivesHelpers.h"
+
+#include "ActsGeometryInterfaces/DetectorAlignStore.h"
+#include "ActsGeometryInterfaces/ActsGeometryContext.h"
+#include "ActsGeometryInterfaces/IDetectorElement.h"
+
+#include "ActsGeoUtils/TransformCache.h"
+#include <stdlib.h>
+
+#include "GeoModelKernel/GeoAlignableTransform.h"
+#include "GeoModelKernel/GeoBox.h"
+#include "GeoModelKernel/GeoFullPhysVol.h"
+#include "GeoModelKernel/GeoPhysVol.h"
+#include "GeoModelKernel/GeoVDetectorElement.h"
+#include "GeoModelHelpers/defineWorld.h"
+
+#include <thread>
+#include <future>
+#include <chrono>
+#include <random>
+
+class TestDetElement : public ActsTrk::IDetectorElement, public GeoVDetectorElement{
+    public:
+        TestDetElement(GeoIntrusivePtr<GeoVFullPhysVol> detVol):
+            GeoVDetectorElement(detVol) { }
+
+        Identifier identify() const override final { 
+            return Identifier{};
+        }
+        ActsTrk::DetectorType detectorType() const  override final { 
+            return ActsTrk::DetectorType::Csc; 
+        }        
+        unsigned int storeAlignedTransforms(const ActsTrk::DetectorAlignStore& store) const override final {
+            m_cache.getTransform(&store);
+            return 1;
+        }
+        const Amg::Transform3D& transform(const Acts::GeometryContext& gctx) const override final {
+            return m_cache.transform(gctx);
+        }
+        const Acts::Surface& surface() const override final  {
+            static const std::shared_ptr<Acts::Surface> surf{};
+            return *surf;
+        }
+        Acts::Surface& surface() override final {
+            static const std::shared_ptr<Acts::Surface> surf{};
+            return *surf;
+        }
+        double thickness() const override final { return 0.;}
+
+    private:
+        ActsTrk::TransformCacheDetEle<TestDetElement> m_cache{IdentifierHash{1}, this};
+};
+
+template<> Amg::Transform3D 
+    ActsTrk::TransformCacheDetEle<TestDetElement>::fetchTransform(const ActsTrk::DetectorAlignStore* store) const {
+    return m_parent->getMaterialGeom()->getAbsoluteTransform(store->geoModelAlignment.get()) * Amg::getRotateX3D(M_PI);
+}
+
+/** Returns how many threads have not yet finished their work*/
+template <class T> size_t count_active(const std::vector<std::future<T>>& threads) {
+    size_t counts{0};
+    for (const std::future<T>& thread : threads) {
+         using namespace std::chrono_literals;
+        if (thread.wait_for(0ms) != std::future_status::ready) ++counts;
+    }
+    return counts;
+}
+
+
+class WorkerTask {
+    public:
+        WorkerTask(const std::vector<std::shared_ptr<TestDetElement>>& detElements,
+                   std::shared_ptr<GeoAlignmentStore> condAlignment):
+            m_detEles{detElements} {
+            // m_store->geoModelAlignment = condAlignment;
+            m_store->geoModelAlignment = std::make_unique<GeoAlignmentStore>(*condAlignment);
+            m_store->geoModelAlignment->clearPosCache();
+        }
+        bool execute() {
+            ActsGeometryContext gctx{};
+            gctx.setStore(m_store);
+            Amg::Transform3D combinedTrf{Amg::Transform3D::Identity()};
+            for (unsigned int daemon = 0 ; daemon < 666; ++daemon) {
+                for (const auto& det : m_detEles) {
+                    const Amg::Transform3D& trf{det->transform(gctx.context())};
+                    // assert(trf.data()[0] != 624626.56);
+
+                    combinedTrf = trf * combinedTrf;
+                    Amg::Vector3D vec = trf.inverse() * Amg::Vector3D{51515,6262,7272};
+                    
+                    if (vec.dot(vec) < 0. || std::pow(vec.eta() * vec.theta(), 2) < 0.) {
+                            std::cout<<"Confusion "<<std::endl;
+                    }
+                }           
+            }
+            m_executed = true;
+            return true;
+        }
+        bool executed() const { return m_executed; }
+    private:
+        std::vector<std::shared_ptr<TestDetElement>> m_detEles{};
+        std::shared_ptr<ActsTrk::DetectorAlignStore> m_store{std::make_unique<ActsTrk::DetectorAlignStore>(ActsTrk::DetectorType::Csc)};
+        bool m_executed{false};
+
+};
+
+class WorkerNode {
+    public:
+        WorkerNode() = default;
+
+        bool hasTask() const {
+            std::shared_lock guard{m_mutex};
+            return m_hasTask;
+        }
+        void assignTask(std::unique_ptr<WorkerTask> task) {        
+            if (hasTask()) return;
+            std::unique_lock guard {m_mutex};
+            m_task = std::move(task);
+            m_hasTask = true;
+        }
+        bool executeTask() {
+            while (!m_abort) {                
+                if (m_task) {
+                    m_task->execute();
+                    std::unique_lock guard{m_mutex};
+                    m_task.reset();
+                    m_hasTask = false;
+                } else {
+                    std::this_thread::sleep_for(std::chrono::milliseconds(10));
+                }
+            }
+            return true;
+        }
+        void finalize() {
+            m_abort = true; 
+        }
+    private:
+        mutable std::shared_mutex m_mutex{};
+        bool m_hasTask{false};
+        std::atomic<bool> m_abort{false};
+        std::unique_ptr<WorkerTask> m_task{};
+};
+
+
+int main() {
+    
+    unsigned int nThreads = std::min(8u, std::thread::hardware_concurrency());
+    constexpr unsigned int numTrials = 40;
+
+    /* Define the world and place randomly the volumes */
+    PVLink world{createGeoWorld()};
+    
+    /** Define the GeoAlignmentStore to move the volumes coherently */
+    std::shared_ptr<GeoAlignmentStore> condAlignment = std::make_shared<GeoAlignmentStore>();
+   
+    std::vector<std::shared_ptr<TestDetElement>> detElements{};
+    
+    for (unsigned int k =0 ; k < 25; ++k) {
+        GeoIntrusivePtr<GeoAlignableTransform> alignTrf = make_intrusive<GeoAlignableTransform>(Amg::getTranslateX3D(k+1));
+        condAlignment->setDelta(alignTrf, Amg::getTranslateY3D(k+1) * Amg::getRotateX3D(M_PI_2));
+        world->add(alignTrf);
+        
+        GeoIntrusivePtr<GeoPhysVol> alignBox = make_intrusive<GeoPhysVol>(world->getLogVol());
+        world->add(alignBox);
+        for (unsigned int d = 0 ; d < 66; ++d) {
+            world->add(make_intrusive<GeoTransform>(Amg::getTranslateZ3D(d+6)));
+            GeoIntrusivePtr<GeoFullPhysVol> detVol{make_intrusive<GeoFullPhysVol>(world->getLogVol())};
+            world->add(detVol);
+            detElements.emplace_back(std::make_unique<TestDetElement>(detVol));
+        }
+    }
+    condAlignment->lockDelta();
+
+    std::vector<std::unique_ptr<WorkerNode>> workers{};
+    std::vector<std::future<bool>> threads{};
+    
+    for (unsigned int node = 0 ; node < nThreads; ++node) {
+        workers.emplace_back(std::make_unique<WorkerNode>());
+        WorkerNode* testerPtr = workers.back().get();
+        threads.emplace_back(std::async(std::launch::async,[testerPtr](){return testerPtr->executeTask();}));
+
+    }
+    
+    std::random_device rd;
+    std::mt19937 g(rd());
+
+    unsigned int executedAttempts{};
+    while (executedAttempts < numTrials) {
+        for (auto& worker : workers) {
+            if (!worker->hasTask()) {
+                worker->assignTask(std::make_unique<WorkerTask>(detElements, condAlignment));
+                std::cout<<"Launch new worker "<<executedAttempts<<" "<<std::endl;
+                if (executedAttempts % 100 == 0) std::shuffle(detElements.begin(),detElements.end(), g);
+                ++executedAttempts;
+            }
+        }
+        std::this_thread::sleep_for(std::chrono::nanoseconds(100));
+    }
+    for (auto & worker : workers) {
+        worker->finalize();
+    }
+    while (count_active(threads) > 0){
+        std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    }
+
+    return EXIT_SUCCESS;
+}
+

Tracking/Acts/ActsGeoUtils/test/test_TicketAlignStore.cxx

-
-
+/*
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+*/
 #include "ActsGeometryInterfaces/DetectorAlignStore.h"
 
 #include <stdlib.h>