From 8f32e69016e25922f419dc9be928c8d4b3d17cef Mon Sep 17 00:00:00 2001
From: Markus Frank <Markus.Frank@cern.ch>
Date: Thu, 2 Mar 2017 17:19:43 +0100
Subject: [PATCH] Remove unneded member data from volume manager

---
 .../DD4hep/objects/VolumeManagerInterna.h     | 15 +---
 DDCore/src/VolumeManager.cpp                  | 49 ++++++++----
 DDCore/src/VolumeManagerInterna.cpp           |  2 +-
 DDCore/src/plugins/VolumeMgrTest.cpp          | 77 ++++++++++---------
 4 files changed, 78 insertions(+), 65 deletions(-)

diff --git a/DDCore/include/DD4hep/objects/VolumeManagerInterna.h b/DDCore/include/DD4hep/objects/VolumeManagerInterna.h
index 4da30cea5..331799ed3 100644
--- a/DDCore/include/DD4hep/objects/VolumeManagerInterna.h
+++ b/DDCore/include/DD4hep/objects/VolumeManagerInterna.h
@@ -51,11 +51,8 @@ namespace DD4hep {
      */
     class VolumeManagerContext {
     public:
-      typedef std::vector<TGeoNode*> Path;
-      typedef PlacedVolume::VolIDs::Base VolIDs;
-
       /// Placement identifier
-      VolumeID identifier;
+      VolumeID     identifier;
       /// Ignore mask of the placement identifier
       [[gnu::deprecated("This member variable might get axed if it is not used, please tell us if you do")]]
       VolumeID mask;
@@ -66,18 +63,14 @@ namespace DD4hep {
       [[gnu::deprecated("This member variable might get axed if it is not used, please tell us if you do")]]
       DetElement detector;
       /// Handle to the closest Detector element
-      DetElement element;
+      DetElement   element;
+      /// The transformation of space-points to the world corrdinate system
+      TGeoHMatrix  toWorld;
       /// The transformation of space-points to the corrdinate system of the closests detector element
       [[gnu::deprecated("This member variable might get axed if it is not used, please tell us if you do")]]
       TGeoHMatrix toDetector;
       /// The transformation of space-points to the world corrdinate system
       TGeoHMatrix toWorld;
-      /// Volume IDS corresponding to this element
-      [[gnu::deprecated("This member variable might get axed if it is not used, please tell us if you do")]]
-      VolIDs volID;
-      /// Path of placements to this sensitive volume
-      [[gnu::deprecated("This member variable might get axed if it is not used, please tell us if you do")]]
-      Path path;
     public:
       /// Default constructor
       VolumeManagerContext();
diff --git a/DDCore/src/VolumeManager.cpp b/DDCore/src/VolumeManager.cpp
index 5fd9b37bc..3c57df3ed 100644
--- a/DDCore/src/VolumeManager.cpp
+++ b/DDCore/src/VolumeManager.cpp
@@ -313,17 +313,24 @@ namespace DD4hep {
             context->detector   = parent;
             context->placement  = PlacedVolume(n);
             context->element    = e;
-            //context->volID      = ids;
-            context->path       = nodes;
             for (size_t i = nodes.size(); i > 1; --i) {   // Omit the placement of the parent DetElement
               TGeoMatrix* m = nodes[i-1]->GetMatrix();
               context->toWorld.MultiplyLeft(m);
             }
+            //            context->volID      = ids;
+            //            context->path       = nodes;
             context->toDetector = context->toWorld;
             context->toDetector.MultiplyLeft(nodes[0]->GetMatrix());
-            //context->toWorld.MultiplyLeft(&parent->worldTransformation());
             context->toWorld.MultiplyLeft(&parent.nominal().worldTransformation());
+            if (!section.adoptPlacement(context)) {
+              print_node(sd, parent, e, n, code, nodes);
+            }
+            m_entries.insert(code.first);
 
+            /** Comment this section to avoid too many computations .... 
+             *  These are only consistentcyu tests
+             */
+            
             // We HAVE to check at least once if the matrices from the original DetElement
             // and from the nominal alignment are identical....
             string p = "";
@@ -409,10 +416,6 @@ namespace DD4hep {
                 }
               }
             }
-            if (!section.adoptPlacement(context)) {
-              print_node(sd, parent, e, n, code, nodes);
-            }
-            m_entries.insert(code.first);
           }
         }
       }
@@ -436,8 +439,10 @@ namespace DD4hep {
             context->detector   = parent;
             context->placement  = PlacedVolume(n);
             context->element    = e;
+#if 0
             context->volID      = ids;
             context->path       = nodes;
+#endif
             for (size_t i = nodes.size(); i > 1; --i) {   // Omit the placement of the parent DetElement
               TGeoMatrix* m = nodes[i - 1]->GetMatrix();
               context->toWorld.MultiplyLeft(m);
@@ -445,7 +450,6 @@ namespace DD4hep {
             context->toDetector = context->toWorld;
             context->toDetector.MultiplyLeft(nodes[0]->GetMatrix());
             context->toWorld.MultiplyLeft(&parent.worldTransformation());
-            //context->toWorld.MultiplyLeft(&parent.nominal().worldTransformation());
             if (!section.adoptPlacement(context)) {
               print_node(sd, parent, e, n, ids, nodes);
             }
@@ -636,45 +640,53 @@ bool VolumeManager::adoptPlacement(VolumeID /* sys_id */, Context* context) {
   VolumeID vid = context->identifier;
   PlacedVolume pv = context->placement;
   Volumes::iterator i = o.volumes.find(vid);
-#if 0
+
   if ( (context->identifier&context->mask) != context->identifier ) {
-    err << "Bad context mask:" << (void*)context->mask << " id:" << (void*)context->identifier
+    err << "Bad context mask:" << (void*)context->mask
+        << " id:" << (void*)context->identifier
         << " pv:" << pv.name() << " Sensitive:"
         << yes_no(pv.volume().isSensitive()) << endl;
     goto Fail;
   }
-#endif
 
   if (i == o.volumes.end()) {
     o.volumes[vid] = context;
     o.detMask |= context->mask;
-    err << "Inserted new volume:" << setw(6) << left << o.volumes.size() << " Ptr:" << (void*) pv.ptr() << " ["
-        << pv.name() << "]" << " ID:" << (void*) context->identifier << " Mask:" << (void*) context->mask;
+    err << "Inserted new volume:" << setw(6) << left << o.volumes.size()
+        << " Ptr:" << (void*) pv.ptr()
+        << " ["    << pv.name() << "]"
+        << " ID:" << (void*) context->identifier
+        << " Mask:" << (void*) context->mask;
     printout(VERBOSE, "VolumeManager", err.str().c_str());
     return true;
   }
-  err << "+++ Attempt to register duplicate volID " << (void*) context->identifier
+  err << "+++ Attempt to register duplicate"
+      << " volID " << (void*) context->identifier
       << " Mask:" << (void*) context->mask
       << " to detector " << o.detector.name()
       << " ptr:" << (void*) pv.ptr()
       << " Name:" << pv.name()
       << " Sensitive:" << yes_no(pv.volume().isSensitive()) << endl;
   printout(ERROR, "VolumeManager", "%s", err.str().c_str());
+#if 0
   err.str("");
   err << " !!!!!                      ++++ VolIDS ";
   const VolIDs::Base& id_vector = context->volID;
   for (VolIDs::Base::const_iterator vit = id_vector.begin(); vit != id_vector.end(); ++vit)
     err << (*vit).first << "=" << (*vit).second << "; ";
   printout(ERROR, "VolumeManager", "%s", err.str().c_str());
+#endif
   err.str("");
   context = (*i).second;
   pv = context->placement;
-  err << " !!!!!               +++ Clashing volID " << (void*) context->identifier
+  err << " !!!!!               +++ Clashing"
+      << " volID " << (void*) context->identifier
       << " Mask:" << (void*) context->mask
       << " to detector " << o.detector.name()
       << " ptr:" << (void*) pv.ptr()
       << " Name:" << pv.name()
       << " Sensitive:" << yes_no(pv.volume().isSensitive()) << endl;
+#if 0
   printout(ERROR, "VolumeManager", "%s", err.str().c_str());
   err.str("");
 
@@ -685,6 +697,8 @@ bool VolumeManager::adoptPlacement(VolumeID /* sys_id */, Context* context) {
     for (VolIDs::Base::const_iterator vit = ids.begin(); vit != ids.end(); ++vit)
       err << (*vit).first << "=" << (*vit).second << "; ";
   }
+#endif
+ Fail:
   printout(ERROR, "VolumeManager", "%s", err.str().c_str());
   // throw runtime_error(err.str());
   return false;
@@ -803,8 +817,9 @@ std::ostream& DD4hep::Geometry::operator<<(std::ostream& os, const VolumeManager
     const VolumeManager::Context* c = (*i).second;
     PlacedVolume pv = c->placement;
     os << prefix << "PV:" << setw(32) << left << pv.name() 
-       << " id:" << setw(18) << left << (void*) c->identifier << " mask:"
-       << setw(18) << left << (void*) c->mask << endl;
+       << " id:"   << setw(18) << left << (void*) c->identifier
+       << " mask:" << setw(18) << left << (void*) c->mask
+       << endl;
   }
   for (VolumeManager::Managers::const_iterator i = o.managers.begin(); i != o.managers.end(); ++i)
     os << prefix << (*i).second << endl;
diff --git a/DDCore/src/VolumeManagerInterna.cpp b/DDCore/src/VolumeManagerInterna.cpp
index 8e0d0f05e..4c04513e3 100644
--- a/DDCore/src/VolumeManagerInterna.cpp
+++ b/DDCore/src/VolumeManagerInterna.cpp
@@ -23,7 +23,7 @@ DD4HEP_INSTANTIATE_HANDLE_NAMED(VolumeManagerObject);
 
 /// Default constructor
 VolumeManagerContext::VolumeManagerContext()
-  : identifier(0), mask(~0x0ULL) {
+ : identifier(0), mask(~0x0ULL) {
 }
 
 /// Default destructor
diff --git a/DDCore/src/plugins/VolumeMgrTest.cpp b/DDCore/src/plugins/VolumeMgrTest.cpp
index 3adef8226..50da8b29c 100644
--- a/DDCore/src/plugins/VolumeMgrTest.cpp
+++ b/DDCore/src/plugins/VolumeMgrTest.cpp
@@ -77,7 +77,8 @@ VolIDTest::VolIDTest(LCDD& lcdd, DetElement sdet, size_t depth) : m_det(sdet) {
     err << "The sensitive detector of subdetector " << m_det.name()
         << " is not known to the geometry.";
     printout(INFO,"VolIDTest",err.str().c_str());
-    throw runtime_error(err.str());
+    //throw runtime_error(err.str());
+    return;
   }
   m_iddesc = lcdd.sensitiveDetector(m_det.name()).readout().idSpec();
   walk(m_det,VolIDs(),0,depth);
@@ -86,45 +87,49 @@ VolIDTest::VolIDTest(LCDD& lcdd, DetElement sdet, size_t depth) : m_det(sdet) {
 /// Check volume integrity
 void VolIDTest::checkVolume(DetElement e, PlacedVolume pv, const VolIDs& child_ids)  const {
   stringstream err, log;
-  VolumeID vid = m_iddesc.encode(child_ids);
-  try {
-    DetElement   det       = m_mgr.lookupDetector(vid);
-    DetElement   det_elem  = m_mgr.lookupDetElement(vid);
-    PlacedVolume det_place = m_mgr.lookupPlacement(vid);
-    if ( pv.ptr() != det_place.ptr() )   {
-      err << "Wrong placement "
-          << " got "        << det_place.name() << " (" << (void*)det_place.ptr() << ")"
-          << " instead of " << pv.name()        << " (" << (void*)pv.ptr()        << ") "
-          << " vid:" << (void*)vid;
+  bool is_sensitive = pv.volume().isSensitive();
+  if ( is_sensitive )  {
+    VolumeID vid = m_iddesc.encode(child_ids);
+    try {
+      DetElement   det       = m_mgr.lookupDetector(vid);
+      DetElement   det_elem  = m_mgr.lookupDetElement(vid);
+      PlacedVolume det_place = m_mgr.lookupPlacement(vid);
+      if ( pv.ptr() != det_place.ptr() )   {
+        err << "Wrong placement "
+            << " got "        << det_place.name() << " (" << (void*)det_place.ptr() << ")"
+            << " instead of " << pv.name()        << " (" << (void*)pv.ptr()        << ") "
+            << " vid:" << (void*)vid;
+      }
+      else if ( det_elem.ptr() != e.ptr() )   {
+        err << "Wrong associated detector element vid="  << (void*)vid
+            << " got "        << det_elem.path() << " (" << (void*)det_elem.ptr() << ") "
+            << " instead of " << e.path()        << " (" << (void*)e.ptr()        << ")"
+            << " vid:" << (void*)vid;
+      }
+      else if ( det.ptr() != m_det.ptr() )   {
+        err << "Wrong associated detector "
+            << " vid:" << (void*)vid;
+      }
     }
-    else if ( det_elem.ptr() != e.ptr() )   {
-      err << "Wrong associated detector element vid="  << (void*)vid
-          << " got "        << det_elem.path() << " (" << (void*)det_elem.ptr() << ") "
-          << " instead of " << e.path()        << " (" << (void*)e.ptr()        << ")"
-          << " vid:" << (void*)vid;
+    catch(const exception& ex) {
+      err << "Lookup " << pv.name() << " id:" << (void*)vid << " path:" << e.path() << " error:" << ex.what();
     }
-    else if ( det.ptr() != m_det.ptr() )   {
-      err << "Wrong associated detector "
-          << " vid:" << (void*)vid;
+    const IDDescriptor::FieldMap& m = m_iddesc.fields();
+    log << "IDS(" << pv.name() << ") ";
+    log << " vid:" << setw(16) << hex << setfill('0') << vid << dec << setfill(' ') << " ";
+    for ( size_t fi=0; fi<m.size(); ++fi )    {
+      IDDescriptor::Field fld = m_iddesc.field(fi);
+      long val = fld->value(vid);
+      if ( find_if(child_ids.begin(),child_ids.end(),FND(fld->name())) == child_ids.end() ) continue;
+      log << fld->name() << (val>=0?": ":":") << val << "  ";
     }
+    if ( !err.str().empty() )   {
+      printout(ERROR,m_det.name(),err.str()+" "+log.str());
+      //throw runtime_error(err.str());
+      return;
+    }
+    printout(INFO,m_det.name(),"OK "+log.str());
   }
-  catch(const exception& ex) {
-    err << "Lookup " << pv.name() << " id:" << (void*)vid << " path:" << e.path() << " error:" << ex.what();
-  }
-  const IDDescriptor::FieldMap& m = m_iddesc.fields();
-  log << "IDS(" << pv.name() << ") ";
-  log << " vid:" << setw(16) << hex << setfill('0') << vid << dec << setfill(' ') << " ";
-  for ( size_t fi=0; fi<m.size(); ++fi )    {
-    IDDescriptor::Field fld = m_iddesc.field(fi);
-    long val = fld->value(vid);
-    if ( find_if(child_ids.begin(),child_ids.end(),FND(fld->name())) == child_ids.end() ) continue;
-    log << fld->name() << (val>=0?": ":":") << val << "  ";
-  }
-  if ( !err.str().empty() )   {
-    printout(ERROR,m_det.name(),err.str()+" "+log.str());
-    throw runtime_error(err.str());
-  }
-  printout(INFO,m_det.name(),"OK "+log.str());
 }
 
 /// Walk through tree of detector elements
-- 
GitLab