Merge pull request #305 from eic/davidl_deadlock_fix

Deadlock Fix

src/algorithms/calorimetry/CalorimeterHitReco.cc

@@ -35,6 +35,16 @@ void CalorimeterHitReco::AlgorithmInit(std::shared_ptr<spdlog::logger>& logger)
         return;
     }
 
+    // First, try and get the IDDescriptor. This will throw an exception if it fails.
+    try{
+        auto id_spec = m_geoSvc->detector()->readout(m_readout).idSpec();
+    }catch(...){
+        m_logger->warn("Failed to get idSpec for {}", m_readout);
+        return;
+    }
+
+    // Get id_spec again, but here it should always succeed.
+    // TODO: This is a bit of a hack so should be cleaned up.
     auto id_spec = m_geoSvc->detector()->readout(m_readout).idSpec();
     try {
         id_dec = id_spec.decoder();
@@ -106,6 +116,16 @@ void CalorimeterHitReco::AlgorithmChangeRun() {
 //------------------------
 void CalorimeterHitReco::AlgorithmProcess() {
 
+    // For some detectors, the cellID in the raw hits may be broken
+    // (currently this is the HcalBarrel). In this case, dd4hep
+    // prints an error message and throws an exception. We catch
+    // the exception and handle it, but the screen gets flooded
+    // with these messages. Keep a count of these and if a max
+    // number is encountered disable this algorithm. A useful message
+    // indicating what is going on is printed below where the
+    // error is detector.
+    if( NcellIDerrors >= MaxCellIDerrors) return;
+
     auto converter = m_geoSvc->cellIDPositionConverter();
     for (const auto rh: rawhits) {
 //        #pragma GCC diagnostic push
@@ -129,7 +149,6 @@ void CalorimeterHitReco::AlgorithmProcess() {
                 id_dec != nullptr && !m_layerField.empty() ? static_cast<int>(id_dec->get(cellID, layer_idx)) : -1;
         const int sid =
                 id_dec != nullptr && !m_sectorField.empty() ? static_cast<int>(id_dec->get(cellID, sector_idx)) : -1;
-
         dd4hep::Position gpos;
         try {
             // global positions
@@ -141,8 +160,15 @@ void CalorimeterHitReco::AlgorithmProcess() {
                 local = volman.lookupDetElement(cellID & local_mask);
             }
         } catch (...) {
-            // Error looking up cellID. Messages should already have been printed
-            // so just skip this hit. User will decide what to do with error messages
+            // Error looking up cellID. Messages should already have been printed.
+            // Also, see comment at top of this method.
+            if( ++NcellIDerrors >= MaxCellIDerrors ){
+                m_logger->error("Maximum number of errors reached: {}", MaxCellIDerrors);
+                m_logger->error("This is likely an issue with the cellID being unknown.");
+                m_logger->error("Note: local_mask={:X} example cellID={:x}", local_mask, cellID);
+                m_logger->error("Disabling this algorithm since it requires a valid cellID.");
+                m_logger->error("(See {}:{})", __FILE__,__LINE__);
+            }
             continue;
         }
 

src/algorithms/calorimetry/CalorimeterHitReco.h

@@ -70,6 +70,8 @@ class CalorimeterHitReco {
   std::string m_sectorField="sectorField";
 
   dd4hep::BitFieldCoder* id_dec = nullptr;
+  uint32_t NcellIDerrors = 0;
+  uint32_t MaxCellIDerrors = 100;
 
   size_t sector_idx{0}, layer_idx{0};
 

src/detectors/HCAL/CalorimeterHit_factory_HcalBarrelRecHits.h

@@ -45,7 +45,7 @@ class CalorimeterHit_factory_HcalBarrelRecHits : public JFactoryT<edm4eic::Calor
         m_sectorField="sector";      
 
         m_localDetElement="";         
-        u_localDetFields={};          
+        u_localDetFields={};
 
 //        app->SetDefaultParameter("HCAL:tag",              m_input_tag);
         app->SetDefaultParameter("HCAL:HcalBarrelRecHits:capacityADC",      m_capADC);
@@ -65,7 +65,7 @@ class CalorimeterHit_factory_HcalBarrelRecHits : public JFactoryT<edm4eic::Calor
         m_geoSvc = app->template GetService<JDD4hep_service>(); // TODO: implement named geometry service?
 
         std::string tag=this->GetTag();
-        std::shared_ptr<spdlog::logger> m_log = app->GetService<Log_service>()->logger(tag);
+        m_log = app->GetService<Log_service>()->logger(tag);
 
         // Get log level from user parameter or default
         std::string log_level_str = "info";
@@ -95,6 +95,7 @@ class CalorimeterHit_factory_HcalBarrelRecHits : public JFactoryT<edm4eic::Calor
         hits.clear(); // not really needed, but better to not leave dangling pointers around
     }
 
+    std::shared_ptr<spdlog::logger> m_log;
 };
 
 #endif // CalorimeterHit_factory_HcalBarrelRecHits_h_

src/detectors/HCAL/ProtoCluster_factory_HcalBarrelTruthProtoClusters.h

@@ -10,7 +10,8 @@
 #include <JANA/JFactoryT.h>
 #include <services/geometry/dd4hep/JDD4hep_service.h>
 #include <algorithms/calorimetry/CalorimeterTruthClustering.h>
-
+#include <services/log/Log_service.h>
+#include <extensions/spdlog/SpdlogExtensions.h>
 
 
 class ProtoCluster_factory_HcalBarrelTruthProtoClusters : public JFactoryT<edm4eic::ProtoCluster>, CalorimeterTruthClustering {
@@ -29,6 +30,8 @@ class ProtoCluster_factory_HcalBarrelTruthProtoClusters : public JFactoryT<edm4e
         m_inputHit_tag="HcalBarrelRecHits";
         m_inputMCHit_tag="HcalBarrelHits";
 
+        m_log = app->GetService<Log_service>()->logger("HcalBarrelTruthProtoClusters");
+
         AlgorithmInit();
     }
 
@@ -56,6 +59,8 @@ class ProtoCluster_factory_HcalBarrelTruthProtoClusters : public JFactoryT<edm4e
     // Name of input data type (collection)
     std::string              m_inputHit_tag;
     std::string              m_inputMCHit_tag;
+
+    std::shared_ptr<spdlog::logger> m_log;
 };
 
 #endif // _ProtoCLuster_factory_HcalBarrelIslandProtoClusters_h_