Fix typo in PhysVolSorter & add more athena tests

GeoModelCore/GeoModelHelpers/GeoModelHelpers/GeoDeDuplicator.h

@@ -78,13 +78,23 @@ class GeoDeDuplicator {
         GeoLogVolPtr cacheVolume(GeoLogVolPtr vol) const;
         GeoShapePtr cacheShape(GeoShapePtr shape) const;
 
+        void setShapeDeduplication(bool enable);
+        void setLogVolDeDuplication(bool enable);
+        void setTransformDeDuplication(bool enable);
+        void setPhysVolDeDuplication(bool enable);
+
     private:
+        bool m_deDuplicateLogVol{true};
+        bool m_deDuplicatePhysVol{true};
+        bool m_deDuplicateTransform{true};
+        bool m_deDuplicateShape{true};
         using PhysVolSet = std::set<GeoIntrusivePtr<GeoPhysVol>, GeoPhysVolSorter>;
         using LogVolSet = std::set<GeoLogVolPtr, GeoLogVolSorter>;
         using TrfSet = std::set<GeoTrfPtr, GeoTrf::TransformSorter>;
         using ShapeSet = std::set<GeoShapePtr, GeoShapeSorter>;
         mutable PhysVolSet m_physVolStore{};
         mutable LogVolSet m_logVolStore{};
+        mutable std::vector<GeoIntrusivePtr<const RCBase>> m_genericCache{};
 
         static TrfSet s_trfStore;
         static ShapeSet s_shapeStore;

GeoModelCore/GeoModelHelpers/src/GeoDeDuplicator.cxx

@@ -8,19 +8,50 @@
 GeoDeDuplicator::TrfSet GeoDeDuplicator::s_trfStore{};
 GeoDeDuplicator::ShapeSet GeoDeDuplicator::s_shapeStore{};
 
+
+void GeoDeDuplicator::setShapeDeduplication(bool enable){
+    m_deDuplicateShape = enable;
+}
+void GeoDeDuplicator::setLogVolDeDuplication(bool enable) {
+    m_deDuplicateLogVol = enable;
+}
+void GeoDeDuplicator::setTransformDeDuplication(bool enable) {
+    m_deDuplicateTransform = enable;
+}
+void GeoDeDuplicator::setPhysVolDeDuplication(bool enable) {
+    m_deDuplicatePhysVol = enable;
+}
+
 GeoDeDuplicator::GeoTrfPtr 
     GeoDeDuplicator::makeTransform(const GeoTrf::Transform3D& trf) const {
-        return *s_trfStore.emplace(new GeoTransform(trf)).first;
+        GeoTrfPtr trfNode{new GeoTransform(trf)};
+        if (!m_deDuplicateTransform) {
+            m_genericCache.push_back(trfNode);
+            return trfNode;
+        }
+        return *s_trfStore.emplace(trfNode).first;
     }
 GeoDeDuplicator::GeoPhysVolPtr 
     GeoDeDuplicator::cacheVolume(GeoPhysVolPtr vol) const {
+        if (!m_deDuplicatePhysVol) {
+            m_genericCache.push_back(vol);
+            return vol;
+        }
         return *m_physVolStore.insert(vol).first;
     }
 GeoDeDuplicator::GeoLogVolPtr 
     GeoDeDuplicator::cacheVolume(GeoLogVolPtr vol) const {
+        if (!m_deDuplicateLogVol) {
+            m_genericCache.push_back(vol);
+            return vol;
+        }
         return *m_logVolStore.insert(vol).first;
     }
 GeoDeDuplicator::GeoShapePtr 
     GeoDeDuplicator::cacheShape(GeoShapePtr shape) const {
+        if (!m_deDuplicateShape) {
+            m_genericCache.push_back(shape);
+            return shape;
+        } 
         return *s_shapeStore.insert(shape).first;
     }

GeoModelCore/GeoModelHelpers/src/GeoPhysVolSorter.cxx

@@ -8,13 +8,15 @@
 #include "GeoModelHelpers/GeoLogVolSorter.h"
 #include "GeoModelKernel/GeoVolumeCursor.h"
 #include "GeoModelHelpers/getChildNodesWithTrf.h"
+#include "GeoModelHelpers/throwExcept.h"
+
 
 int GeoPhysVolSorter::compare(const GeoVPhysVol* a, const GeoVPhysVol* b) const {
     /// If one of the given volumes is a full physical volume, let's assume that they've been
     /// put on purpose into the node. Ensure that they're added to the set eventhough they
     /// are equivalent in the terms of this sorter.
     if (typeid(*a) == typeid(GeoFullPhysVol) || typeid(*b) == typeid(GeoFullPhysVol)) {
-        return a < b;
+        return a == b ? 0 : (a < b ? -1 : 1);
     }
     
     /// A check on different logical volumes is already a good start
@@ -35,11 +37,11 @@ int GeoPhysVolSorter::compare(const GeoVPhysVol* a, const GeoVPhysVol* b) const
         cursB.next();
         /// Check whether there's an alignable transform somewhere
         if (childA.isAlignable != childB.isAlignable) {
-            return childA.isAlignable;
+            return childA.isAlignable ? -1 : 1;
         }
         /// Check whether the voumes are full physical volumes
         if (childA.isSensitive != childB.isSensitive) {
-            return childA.isSensitive;
+            return childA.isSensitive ? -1 : 1;
         }
         /// Check equivalance of the transformations
         const int transCmp = sorter.compare(childA.transform,

GeoModelCore/GeoModelHelpers/src/TransformSorter.cxx

@@ -2,6 +2,7 @@
   Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 #include "GeoModelHelpers/TransformSorter.h"
+#include "GeoModelKernel/GeoAlignableTransform.h"
 #include "GeoModelHelpers/throwExcept.h"
 
 namespace GeoTrf {
@@ -41,6 +42,10 @@ namespace GeoTrf {
         if (!a || !b) {
             THROW_EXCEPTION("Nullptr given to comparator");
         }
+        if (typeid(*a) == typeid(GeoAlignableTransform) ||
+            typeid(*b) == typeid(GeoAlignableTransform)) {
+            return a < b;
+        }
         return (*this)(a->getTransform(), b->getTransform());
     }
 }
\ No newline at end of file

GeoModelCore/GeoModelHelpers/tests/testPhysVolSorter.cxx

+/*
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+*/
+
+#include "GeoModelHelpers/GeoPhysVolSorter.h"
+#include "GeoModelHelpers/defineWorld.h"
+#include "GeoModelKernel/GeoAlignableTransform.h"
+#include "GeoModelHelpers/TransformToStringConverter.h"
+#include "GeoModelKernel/GeoFullPhysVol.h"
+#include "GeoModelKernel/GeoBox.h"
+#include "GeoModelKernel/Units.h"
+#include <stdlib.h>
+#include <iostream>
+#include <set>
+
+
+int main() {
+    std::set<PVLink, GeoPhysVolSorter> physVolSet{};
+
+    GeoIntrusivePtr<GeoPhysVol> world{createGeoWorld()};
+
+    const GeoMaterial* air = world->getLogVol()->getMaterial();
+    GeoIntrusivePtr<GeoBox> externalBox{new GeoBox(500.,500., 500.)};
+    GeoIntrusivePtr<GeoBox> internalBox{new GeoBox(100.,100., 100.)};
+    auto  makeBox = [&](bool bigOne) {
+        return PVLink(new GeoPhysVol(new GeoLogVol("TestVolume", bigOne ? externalBox : internalBox, air)));
+    };
+
+    auto makeFullBox = [&](bool bigOne) {
+        return PVLink(new GeoFullPhysVol(new GeoLogVol("TestFullPhysVol", bigOne ? externalBox : internalBox, air)));
+    };
+    /// 
+    PVLink extVolume = makeBox(true);
+    extVolume->add(makeBox(false));
+
+    if (!physVolSet.insert(extVolume).second) {
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" Cannot insert the box into the empty set "<<std::endl;
+        return EXIT_FAILURE;
+    }
+    if (!physVolSet.insert(makeBox(true)).second){
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" Cannot inssert the empty big box "<<std::endl;
+        return EXIT_FAILURE;
+    }
+    if (!physVolSet.insert(makeBox(false)).second) {
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" Cannot inssert the empty small box "<<std::endl;
+        return EXIT_FAILURE;
+    }
+
+    /// Test whether a box with a subbox cannot be added
+    extVolume = makeBox(true);
+    extVolume->add(makeBox(false));
+    if (physVolSet.insert(extVolume).second) {
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" PhysVols can be shared. "<<std::endl;
+        return EXIT_FAILURE;
+    }
+    extVolume = makeBox(true);
+    extVolume->add(new GeoTransform(GeoTrf::TranslateX3D(50.)));
+    extVolume->add(makeBox(false));
+    if (!physVolSet.insert(extVolume).second) {
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" A box with a displaced box inside is not the same as box ception "<<std::endl;
+        return EXIT_FAILURE;
+    }
+    /// Test the full physical volumes
+
+    if (!physVolSet.insert(makeFullBox(true)).second) {
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" Full physical volume should be always added. "<<std::endl;
+        return EXIT_FAILURE;
+    }
+    if (!physVolSet.insert(makeFullBox(true)).second) {
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" Full physical volume should be always added. "<<std::endl;
+        return EXIT_FAILURE;
+    }
+    if (!physVolSet.insert(makeFullBox(false)).second) {
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" Full physical volume should be always added. "<<std::endl;
+        return EXIT_FAILURE;
+    }
+    if (!physVolSet.insert(makeFullBox(false)).second) {
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" Full physical volume should be always added. "<<std::endl;
+        return EXIT_FAILURE;
+    }
+
+    extVolume = makeBox(true);
+    extVolume->add(makeFullBox(false));    
+    if (!physVolSet.insert(extVolume).second) {
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" A box with a full physical volume inside should be always added "<<std::endl;
+        return EXIT_FAILURE;
+    }
+    
+    extVolume = makeFullBox(true);
+    extVolume->add(makeFullBox(false));    
+    if (!physVolSet.insert(extVolume).second) {
+        std::cerr<<"testPhysVolSorter() "<<__LINE__<<" A box with a full physical volume inside should be always added "<<std::endl;
+        return EXIT_FAILURE;
+    }
+
+
+
+    return EXIT_SUCCESS;
+}
\ No newline at end of file

GeoModelCore/GeoModelHelpers/tests/testTransformSorter.cxx

@@ -3,10 +3,12 @@
 */
 
 #include "GeoModelHelpers/TransformSorter.h"
+#include "GeoModelKernel/GeoAlignableTransform.h"
 #include "GeoModelHelpers/TransformToStringConverter.h"
 #include "GeoModelKernel/Units.h"
 #include <stdlib.h>
 #include <iostream>
+#include <set>
 
 std::ostream& operator<<(std::ostream& ostr, const GeoTrf::Transform3D& trans) {
     ostr<<GeoTrf::toString(trans, true, 2);
@@ -56,6 +58,32 @@ int main() {
     TEST_TRANSFORM(transC)
     COMPARE_SORTER(transA, transC);
     COMPARE_SORTER(transB, transC);
+
+    std::set<GeoIntrusivePtr<GeoTransform>, GeoTrf::TransformSorter> geoTrfCache{};
+    {
+        GeoIntrusivePtr<GeoTransform> geoTransC{new GeoTransform(transC)};
+        if (!geoTrfCache.insert(geoTransC).second) {
+            std::cerr<<"testTransformSorter() "<<__LINE__<<" Failed to insert transform into empty set"<<std::endl;
+            return EXIT_FAILURE;
+        }
+        geoTransC.reset(new GeoTransform(transC));
+        if (geoTrfCache.insert(geoTransC).second) {
+            std::cerr<<"testTransformSorter() "<<__LINE__<<" The same transform should always point to the same object."<<std::endl;
+            return EXIT_FAILURE;
+        }
+        geoTransC.reset(new GeoAlignableTransform(transC));
+        if (!geoTrfCache.insert(geoTransC).second) {
+            std::cerr<<"testTransformSorter() "<<__LINE__<<" Alignable transforms should be never deduplicated."<<std::endl;
+            return EXIT_FAILURE;
+        }
+
+        geoTransC.reset(new GeoAlignableTransform(transC));
+        if (!geoTrfCache.insert(geoTransC).second) {
+            std::cerr<<"testTransformSorter() "<<__LINE__<<" Alignable transforms should be never deduplicated."<<std::endl;
+            return EXIT_FAILURE;
+        }
+
+    }
    
     return EXIT_SUCCESS;
 }
\ No newline at end of file

GeoModelCore/GeoModelKernel/src/GeoDefinitions.cxx

@@ -23,7 +23,7 @@ namespace GeoTrf {
 
   GeoRotation::GeoRotation(double phi, double theta, double psi):
       GeoRotation{EulerAngles{phi, theta, psi}} {}
- 
+
   GeoRotation::GeoRotation(const EulerAngles& angles):
       RotationMatrix3D{get3DRotMatZ(angles.phi) * 
                        get3DRotMatX(angles.theta) *

GeoModelCore/GeoModelKernel/src/GeoShapeShift.cxx

@@ -14,7 +14,7 @@ GeoShapeShift::GeoShapeShift (const GeoShape* A, const GeoTrf::Transform3D &X):
   m_op{A}, m_shift{X} {
     /// Check whether the given shape also a shape shift. If so then we can simply
     /// take the operand of the sub shift and summarize the transformations of the two into one
-   if (A->refCount() > 1 && A->typeID() == typeID()) {
+   if (false && A->refCount() > 1 && A->typeID() == typeID()) {
        const GeoShapeShift* subShift{dynamic_cast<const GeoShapeShift*>(A)};
        GeoTrf::Transform3D updatedShift = m_shift * subShift->getX();
        m_shift = std::move(updatedShift);

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/Element2ShapeItem.h

+/*
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
+*/
+#ifndef GEO_MODEL_XML_ELEMENT_2_SHAPEITEM_H
+#define GEO_MODEL_XML_ELEMENT_2_SHAPEITEM_H
+#include "GeoModelXml/Element2GeoItem.h"
+
+#include <set>
+class Element2ShapeItem: public Element2GeoItem {
+    public:
+        Element2ShapeItem();
+        
+        virtual ~Element2ShapeItem();
+
+        static void deduplicateShapes(const bool enable);
+
+    private:
+        static std::set<Element2ShapeItem*> s_instances;
+        static bool s_applyDepluication;
+};
+
+
+#endif
\ No newline at end of file

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/LogvolProcessor.h

@@ -25,6 +25,8 @@ public:
     } ; 
     void process(const xercesc::DOMElement *element, GmxUtil &gmxUtil, GeoNodeList &toAdd);
     void zeroId(const xercesc::DOMElement *element);
+
+    LogvolProcessor();
 private:
     std::map<std::string, LogVolStore> m_map; 
 };

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/MulticopyProcessor.h

@@ -16,6 +16,7 @@ class GmxUtil;
 
 class MulticopyProcessor: public ElementProcessor {
 public:
+    MulticopyProcessor();
     void process(const xercesc::DOMElement *element, GmxUtil &gmxUtil, GeoNodeList &toAdd);
 private:
     std::map <std::string, GeoNodeList> m_map; 

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/ReplicaRPhiProcessor.h

 /*
-  Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 #ifndef GEO_MODEL_XML_REPLICARPHI_PROCESSOR_H
@@ -16,10 +16,10 @@ class GmxUtil;
 
 class ReplicaRPhiProcessor: public ElementProcessor {
 public:
+    ReplicaRPhiProcessor();
     void process(const xercesc::DOMElement *element, GmxUtil &gmxUtil, GeoNodeList &toAdd);
 private:
-    std::map <std::string, GeoNodeList> m_map; 
-    void tokenize(std::string&,char,std::vector<std::string>&) const ;
+    std::map <std::string, GeoNodeList> m_map{}; 
 };
 
 #endif // REPLICAX_PROCESSOR_H

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/ReplicaXProcessor.h

 /*
-  Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 #ifndef GEO_MODEL_XML_REPLICAX_PROCESSOR_H
@@ -16,10 +16,10 @@ class GmxUtil;
 
 class ReplicaXProcessor: public ElementProcessor {
 public:
+    ReplicaXProcessor();
     void process(const xercesc::DOMElement *element, GmxUtil &gmxUtil, GeoNodeList &toAdd);
 private:
     std::map <std::string, GeoNodeList> m_map; 
-    void tokenize(std::string&,char,std::vector<std::string>&) const ;
 };
 
 #endif // REPLICAX_PROCESSOR_H

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/ReplicaXYArraysProcessor.h

 /*
-  Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 #ifndef GEO_MODEL_XML_REPLICAXYARRAYS_PROCESSOR_H
@@ -16,6 +16,7 @@ class GmxUtil;
 
 class ReplicaXYarraysProcessor: public ElementProcessor {
 public:
+    ReplicaXYarraysProcessor();
     void process(const xercesc::DOMElement *element, GmxUtil &gmxUtil, GeoNodeList &toAdd);
 private:
     std::map <std::string, GeoNodeList> m_map; 

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/ReplicaYProcessor.h

@@ -16,10 +16,10 @@ class GmxUtil;
 
 class ReplicaYProcessor: public ElementProcessor {
 public:
+    ReplicaYProcessor();
     void process(const xercesc::DOMElement *element, GmxUtil &gmxUtil, GeoNodeList &toAdd);
 private:
     std::map <std::string, GeoNodeList> m_map; 
-    void tokenize(std::string&,char,std::vector<std::string>&) const ;
 };
 
 #endif // REPLICAY_PROCESSOR_H

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/ReplicaZProcessor.h

@@ -16,10 +16,10 @@ class GmxUtil;
 
 class ReplicaZProcessor: public ElementProcessor {
 public:
+    ReplicaZProcessor();
     void process(const xercesc::DOMElement *element, GmxUtil &gmxUtil, GeoNodeList &toAdd);
 private:
     std::map <std::string, GeoNodeList> m_map; 
-    void tokenize(std::string&,char,std::vector<std::string>&) const ;
 };
 
 #endif // REPLICAX_PROCESSOR_H

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeBox.h

@@ -10,10 +10,10 @@
 #define GEO_MODEL_XML_MAKE_BOX_H
 #include <xercesc/util/XercesDefs.hpp>
 
-#include "GeoModelXml/Element2GeoItem.h"
+#include "GeoModelXml/Element2ShapeItem.h"
 
 #include "GeoModelKernel/GeoBox.h"
-class MakeBox: public Element2GeoItem {
+class MakeBox: public Element2ShapeItem {
 public:
     MakeBox() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeCons.h

@@ -10,9 +10,9 @@
 #define GEO_MODEL_XML_MAKE_CONS_H
 #include <xercesc/util/XercesDefs.hpp>
 
-#include "GeoModelXml/Element2GeoItem.h"
+#include "GeoModelXml/Element2ShapeItem.h"
 
-class MakeCons: public Element2GeoItem {
+class MakeCons: public Element2ShapeItem {
   public:
       MakeCons() = default;
       virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeEllipticalTube.h

@@ -6,9 +6,9 @@
 #define GEO_MODEL_XML_MAKE_ELLIPTICAL_TUBE_H
 #include <xercesc/util/XercesDefs.hpp>
 
-#include "GeoModelXml/Element2GeoItem.h"
+#include "GeoModelXml/Element2ShapeItem.h"
 
-class MakeEllipticalTube: public Element2GeoItem {
+class MakeEllipticalTube: public Element2ShapeItem {
 public:
     MakeEllipticalTube() = default;;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;

GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeGenericTrap.h

@@ -10,10 +10,10 @@
 #define GEO_MODEL_XML_MAKE_GENERICTRAP_H
 #include <xercesc/util/XercesDefs.hpp>
 
-#include "GeoModelXml/Element2GeoItem.h"
+#include "GeoModelXml/Element2ShapeItem.h"
 
 
-class MakeGenericTrap: public Element2GeoItem {
+class MakeGenericTrap: public Element2ShapeItem {
 public:
     MakeGenericTrap() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;