feat: small detector refinements (#2703)

This PR does two things: - it makes a bit clearer VERBOSE methods - It lets you allow to set a minimum number of surfaces needed to even create a layer structure (and not stay with try all(
acts-project · Nov 21, 2023 · 4921d08 · 4921d08
1 parent 082e64d
commit 4921d08
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 44 deletions.
diff --git a/Core/include/Acts/Detector/LayerStructureBuilder.hpp b/Core/include/Acts/Detector/LayerStructureBuilder.hpp
@@ -86,6 +86,9 @@ class LayerStructureBuilder : public IInternalStructureBuilder {
     std::vector<ProtoSupport> supports = {};
     /// Definition of Binnings
     std::vector<ProtoBinning> binnings = {};
+    /// Minimum number of surfaces to build an internal structure
+    /// - otherwise the tryAll options is used
+    unsigned int nMinimalSurfaces = 4u;
     /// Polyhedron approximations
     unsigned int nSegments = 1u;
     /// Extra information, mainly for screen output

diff --git a/Core/src/Detector/CylindricalContainerBuilder.cpp b/Core/src/Detector/CylindricalContainerBuilder.cpp
@@ -220,10 +220,12 @@ Acts::Experimental::CylindricalContainerBuilder::construct(
     if (m_cfg.geoIdReverseGen) {
       std::for_each(rootVolumes.rbegin(), rootVolumes.rend(), [&](auto& v) {
         m_cfg.geoIdGenerator->assignGeometryId(cache, *v);
+        ACTS_VERBOSE("-> Assigning geometry id to volume " << v->name());
       });
     } else {
       std::for_each(rootVolumes.begin(), rootVolumes.end(), [&](auto& v) {
         m_cfg.geoIdGenerator->assignGeometryId(cache, *v);
+        ACTS_VERBOSE("-> Assigning geometry id to volume " << v->name());
       });
     }
   }

diff --git a/Core/src/Detector/Detector.cpp b/Core/src/Detector/Detector.cpp
@@ -74,7 +74,8 @@ Acts::Experimental::Detector::Detector(
         ss << sgeoID;
         throw std::invalid_argument(
             "Detector: duplicate sensitive surface geometry id '" + ss.str() +
-            "' detected. Make sure a GeometryIdGenerator is used.");
+            "' detected in volume '" + v->name() +
+            "'. Make sure a GeometryIdGenerator is used.");
       }
       surfaceGeoIdMap.emplace(sgeoID, s);
     }

diff --git a/Core/src/Detector/DetectorBuilder.cpp b/Core/src/Detector/DetectorBuilder.cpp
@@ -40,6 +40,7 @@ Acts::Experimental::DetectorBuilder::construct(
     ACTS_DEBUG("Assigning geometry ids to the detector");
     auto cache = m_cfg.geoIdGenerator->generateCache();
     std::for_each(roots.volumes.begin(), roots.volumes.end(), [&](auto& v) {
+      ACTS_VERBOSE("-> Assigning geometry id to volume " << v->name());
       m_cfg.geoIdGenerator->assignGeometryId(cache, *v);
     });
   }

diff --git a/Core/src/Detector/LayerStructureBuilder.cpp b/Core/src/Detector/LayerStructureBuilder.cpp
@@ -214,52 +214,62 @@ Acts::Experimental::LayerStructureBuilder::construct(
           support.values, support.transform, support.splits);
     }
   }
-  if (m_cfg.binnings.empty()) {
-    ACTS_DEBUG(
-        "No surface binning provided, navigation will be 'tryAll' (potentially "
-        "slow).");
-  } else if (m_cfg.binnings.size() == 1u) {
-    ACTS_DEBUG("- 1-dimensional surface binning detected.");
-    // Capture the binning
-    auto binning = m_cfg.binnings[0u];
-    if (binning.boundaryType == Acts::detail::AxisBoundaryType::Closed) {
-      ACTS_VERBOSE("-- closed binning option.");
-      internalCandidatesUpdator =
-          createUpdator<Acts::detail::AxisBoundaryType::Closed>(
-              gctx, internalSurfaces, assignToAll, binning);
-    } else {
-      ACTS_VERBOSE("-- closed binning option.");
-      internalCandidatesUpdator =
-          createUpdator<Acts::detail::AxisBoundaryType::Bound>(
-              gctx, internalSurfaces, assignToAll, binning);
-    }
-  } else if (m_cfg.binnings.size() == 2u) {
-    ACTS_DEBUG("- 2-dimensional surface binning detected.");
-    // Capture the binnings
-    const auto& binning0 = m_cfg.binnings[0u];
-    const auto& binning1 = m_cfg.binnings[1u];
 
-    if (binning0.boundaryType == Acts::detail::AxisBoundaryType::Closed) {
-      ACTS_VERBOSE("-- closed/bound binning option.");
-      internalCandidatesUpdator =
-          createUpdator<Acts::detail::AxisBoundaryType::Closed,
-                        Acts::detail::AxisBoundaryType::Bound>(
-              gctx, internalSurfaces, assignToAll, binning0, binning1);
-    } else if (binning1.boundaryType ==
-               Acts::detail::AxisBoundaryType::Closed) {
-      ACTS_VERBOSE("-- bound/closed binning option.");
-      internalCandidatesUpdator =
-          createUpdator<Acts::detail::AxisBoundaryType::Bound,
-                        Acts::detail::AxisBoundaryType::Closed>(
-              gctx, internalSurfaces, assignToAll, binning0, binning1);
-    } else {
-      ACTS_VERBOSE("-- closed/closed binning option.");
-      internalCandidatesUpdator =
-          createUpdator<Acts::detail::AxisBoundaryType::Bound,
-                        Acts::detail::AxisBoundaryType::Bound>(
-              gctx, internalSurfaces, assignToAll, binning0, binning1);
+  if (internalSurfaces.size() >= m_cfg.nMinimalSurfaces) {
+    if (m_cfg.binnings.empty()) {
+      ACTS_DEBUG(
+          "No surface binning provided, navigation will be 'tryAll' "
+          "(potentially slow).");
+    } else if (m_cfg.binnings.size() == 1u) {
+      ACTS_DEBUG("- 1-dimensional surface binning detected.");
+      // Capture the binning
+      auto binning = m_cfg.binnings[0u];
+      if (binning.boundaryType == Acts::detail::AxisBoundaryType::Closed) {
+        ACTS_VERBOSE("-- closed binning option.");
+        internalCandidatesUpdator =
+            createUpdator<Acts::detail::AxisBoundaryType::Closed>(
+                gctx, internalSurfaces, assignToAll, binning);
+      } else {
+        ACTS_VERBOSE("-- bound binning option.");
+        internalCandidatesUpdator =
+            createUpdator<Acts::detail::AxisBoundaryType::Bound>(
+                gctx, internalSurfaces, assignToAll, binning);
+      }
+    } else if (m_cfg.binnings.size() == 2u) {
+      ACTS_DEBUG("- 2-dimensional surface binning detected.");
+      // Capture the binnings
+      const auto& binning0 = m_cfg.binnings[0u];
+      const auto& binning1 = m_cfg.binnings[1u];
+
+      if (binning0.boundaryType == Acts::detail::AxisBoundaryType::Closed) {
+        ACTS_VERBOSE("-- closed/bound binning option.");
+        internalCandidatesUpdator =
+            createUpdator<Acts::detail::AxisBoundaryType::Closed,
+                          Acts::detail::AxisBoundaryType::Bound>(
+                gctx, internalSurfaces, assignToAll, binning0, binning1);
+      } else if (binning1.boundaryType ==
+                 Acts::detail::AxisBoundaryType::Closed) {
+        ACTS_VERBOSE("-- bound/closed binning option.");
+        internalCandidatesUpdator =
+            createUpdator<Acts::detail::AxisBoundaryType::Bound,
+                          Acts::detail::AxisBoundaryType::Closed>(
+                gctx, internalSurfaces, assignToAll, binning0, binning1);
+      } else {
+        ACTS_VERBOSE("-- bound/bound binning option.");
+        internalCandidatesUpdator =
+            createUpdator<Acts::detail::AxisBoundaryType::Bound,
+                          Acts::detail::AxisBoundaryType::Bound>(
+                gctx, internalSurfaces, assignToAll, binning0, binning1);
+      }
     }
+  } else {
+    ACTS_DEBUG("Only " << internalSurfaces.size() << " surfaces provided, "
+                       << "navigation will be 'tryAll'");
+    ACTS_DEBUG("Per configuration " << m_cfg.nMinimalSurfaces
+                                    << " surfaces are "
+                                    << "required to use the surface binning.");
   }
+
   // Check if everything went ok
   if (!internalCandidatesUpdator.connected()) {
     throw std::runtime_error(