From d5238cec0226e5d525c8ec0528a1c0a864ed309e Mon Sep 17 00:00:00 2001
From: Andre Sailer <andre.philippe.sailer@cern.ch>
Date: Thu, 30 May 2024 18:24:05 +0200
Subject: [PATCH] GeoHandler: make collect give deterministic volume order

---
 DDCore/include/DD4hep/GeoHandler.h |  8 ++++----
 DDCore/src/GeoHandler.cpp          | 14 +++++++++-----
 DDG4/src/Geant4Converter.cpp       |  2 +-
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/DDCore/include/DD4hep/GeoHandler.h b/DDCore/include/DD4hep/GeoHandler.h
index b138d2547..31d507f17 100644
--- a/DDCore/include/DD4hep/GeoHandler.h
+++ b/DDCore/include/DD4hep/GeoHandler.h
@@ -88,7 +88,7 @@ namespace dd4hep {
 
     protected:
       bool  m_propagateRegions { false };
-      std::map<int, std::set<const TGeoNode*> >*    m_data      { nullptr };
+      std::map<int, std::vector<const TGeoNode*> >*    m_data      { nullptr };
       std::map<const TGeoNode*, std::vector<TGeoNode*> >* m_daughters { nullptr };
       /// Internal helper to collect geometry information from traversal
       GeoHandler& i_collect(const TGeoNode* parent,
@@ -108,7 +108,7 @@ namespace dd4hep {
       /// Default constructor
       GeoHandler();
       /// Initializing constructor
-      GeoHandler(std::map<int, std::set<const TGeoNode*> >* ptr,
+      GeoHandler(std::map<int, std::vector<const TGeoNode*> >* ptr,
                  std::map<const TGeoNode*, std::vector<TGeoNode*> >* daus = nullptr);
       /// Default destructor
       virtual ~GeoHandler();
@@ -119,7 +119,7 @@ namespace dd4hep {
       /// Collect geometry information from traversal with aggregated information
       GeoHandler& collect(DetElement top, GeometryInfo& info);
       /// Access to collected node list
-      std::map<int, std::set<const TGeoNode*> >* release();
+      std::map<int, std::vector<const TGeoNode*> >* release();
     };
 
     /// Geometry scanner (handle object)
@@ -131,7 +131,7 @@ namespace dd4hep {
     class GeoScan {
     protected:
       /// Data holder
-      std::map<int, std::set<const TGeoNode*> >* m_data;
+      std::map<int, std::vector<const TGeoNode*> >* m_data;
     public:
       /// Initializing constructor
       GeoScan(DetElement e);
diff --git a/DDCore/src/GeoHandler.cpp b/DDCore/src/GeoHandler.cpp
index 8cd4fee14..ca443d062 100644
--- a/DDCore/src/GeoHandler.cpp
+++ b/DDCore/src/GeoHandler.cpp
@@ -23,6 +23,7 @@
 #include <TClass.h>
 
 // C/C++ include files
+#include <algorithm>
 #include <iostream>
 
 using namespace dd4hep;
@@ -53,11 +54,11 @@ namespace {
 
 /// Default constructor
 detail::GeoHandler::GeoHandler()  {
-  m_data = new std::map<int, std::set<const TGeoNode*> >();
+  m_data = new std::map<int, std::vector<const TGeoNode*> >();
 }
 
 /// Initializing constructor
-detail::GeoHandler::GeoHandler(std::map<int, std::set<const TGeoNode*> >* ptr,
+detail::GeoHandler::GeoHandler(std::map<int, std::vector<const TGeoNode*> >* ptr,
                                std::map<const TGeoNode*, std::vector<TGeoNode*> >* daus)
   : m_data(ptr), m_daughters(daus)
 {
@@ -70,8 +71,8 @@ detail::GeoHandler::~GeoHandler() {
   m_data = nullptr;
 }
 
-std::map<int, std::set<const TGeoNode*> >* detail::GeoHandler::release() {
-  std::map<int, std::set<const TGeoNode*> >* d = m_data;
+std::map<int, std::vector<const TGeoNode*> >* detail::GeoHandler::release() {
+  std::map<int, std::vector<const TGeoNode*> >* d = m_data;
   m_data = nullptr;
   return d;
 }
@@ -149,7 +150,10 @@ detail::GeoHandler& detail::GeoHandler::i_collect(const TGeoNode* /* parent */,
     }
   }
   /// Collect the hierarchy of placements
-  (*m_data)[level].emplace(current);
+  auto& vec = (*m_data)[level];
+  if(std::find(vec.begin(), vec.end(), current) == vec.end()) {
+    (*m_data)[level].push_back(current);
+  }
   int num = nodes ? nodes->GetEntriesFast() : 0;
   for (int i = 0; i < num; ++i)
     i_collect(current, (TGeoNode*)nodes->At(i), level + 1, region, limits);
diff --git a/DDG4/src/Geant4Converter.cpp b/DDG4/src/Geant4Converter.cpp
index f62e8c25f..4ca3bd57e 100644
--- a/DDG4/src/Geant4Converter.cpp
+++ b/DDG4/src/Geant4Converter.cpp
@@ -1689,7 +1689,7 @@ Geant4Converter& Geant4Converter::create(DetElement top) {
   handleRMap(this, *m_data,     &Geant4Converter::handleAssembly);
   // Now place all this stuff appropriately
   //handleRMap(this, *m_data,     &Geant4Converter::handlePlacement);
-  std::map<int, std::set<const TGeoNode*> >::const_reverse_iterator i = m_data->rbegin();
+  std::map<int, std::vector<const TGeoNode*> >::const_reverse_iterator i = m_data->rbegin();
   for ( ; i != m_data->rend(); ++i )  {
     for ( const TGeoNode* node : i->second )  {
 #if 0