Merge c8f7f4e into sapling-pr-archive-ktf

Author: ktf

Detectors/Base/include/DetectorsBase/MatLayerCylSet.h

@@ -87,7 +87,7 @@ class MatLayerCylSet : public o2::gpu::FlatObject
   void flatten();
 
   MatLayerCyl& getLayer(int i) { return get()->mLayers[i]; }
-  MatLayerCylSet* extractCopy(float rmin, float rmax, float tol = 1e-3) const;
+  MatLayerCylSet* extractCopy(float rmin, float rmax, float tol = 1e-3, const MatLayerCylSet* toAdd = nullptr) const;
   void finalizeStructures();
 
 #endif // !GPUCA_ALIGPUCODE

Detectors/Base/src/MatLayerCylSet.cxx

@@ -608,8 +608,12 @@ void MatLayerCylSet::fixPointers(char* oldPtr, char* newPtr, bool newPtrValid)
 
 #ifndef GPUCA_ALIGPUCODE // this part is unvisible on GPU version
 
-MatLayerCylSet* MatLayerCylSet::extractCopy(float rmin, float rmax, float tolerance) const
+MatLayerCylSet* MatLayerCylSet::extractCopy(float rmin, float rmax, float tolerance, const MatLayerCylSet* addTo) const
 {
+  // extract layers in the covering rmin-rmax range. If addTo is provided, simply substitute its layers by those from this
+  if (addTo && addTo->getNLayers() != getNLayers()) {
+    LOGP(fatal, "addTo has {} layers, this has {}", addTo->getNLayers(), getNLayers());
+  }
   Ray ray(std::max(getRMin(), rmin), 0., 0., std::min(getRMax(), rmax), 0., 0.);
   short lmin, lmax;
   if (!getLayersRange(ray, lmin, lmax)) {
@@ -618,23 +622,37 @@ MatLayerCylSet* MatLayerCylSet::extractCopy(float rmin, float rmax, float tolera
   }
   LOGP(info, "Will extract layers {}:{} (out of {} layers) for {} < r < {}", lmin, lmax, getNLayers(), rmin, rmax);
   MatLayerCylSet* copy = new MatLayerCylSet();
-  int lrCount = 0;
-  for (int il = lmin; il <= lmax; il++) {
-    const auto& lr = getLayer(il);
+  int lrCount = 0, lrCounOld = 0, lrCountTot = 0;
+  auto addLr = [copy, &lrCountTot](const MatLayerCyl& lr) {
     float drphi = lr.getDPhi() * (lr.getRMin() + lr.getRMax()) / 2. * 0.999;
     copy->addLayer(lr.getRMin(), lr.getRMax(), lr.getZMax(), lr.getDZ(), drphi);
-    auto& lrNew = copy->getLayer(lrCount);
+    auto& lrNew = copy->getLayer(lrCountTot++);
     for (int iz = 0; iz < lrNew.getNZBins(); iz++) {
       for (int ip = 0; ip < lrNew.getNPhiBins(); ip++) {
         lrNew.getCellPhiBin(ip, iz).set(lr.getCellPhiBin(ip, iz));
       }
     }
+  };
+  if (addTo) {
+    for (int il = 0; il < lmin; il++) {
+      addLr(addTo->getLayer(il));
+      lrCounOld++;
+    }
+  }
+  for (int il = lmin; il <= lmax; il++) {
+    addLr(getLayer(il));
     lrCount++;
   }
-
+  if (addTo) {
+    for (int il = lmax + 1; il < getNLayers(); il++) {
+      addLr(addTo->getLayer(il));
+      lrCounOld++;
+    }
+  }
   copy->finalizeStructures();
   copy->optimizePhiSlices(tolerance);
   copy->flatten();
+  LOGP(info, "Added layers {}:{} for {}<r<{} {}", lmin, lmax, rmin, rmax, fmt::format(", {} layers were transferred from additional set", lrCounOld));
   return copy;
 }
 

Detectors/TPC/base/include/TPCBase/CalArray.h

@@ -26,6 +26,11 @@
 #include <boost/format.hpp>
 #endif
 
+#ifdef NDEBUG
+#undef NDEBUG
+#include <cassert>
+#endif
+
 namespace o2
 {
 namespace tpc
@@ -93,7 +98,11 @@ class CalArray
   int getPadSubsetNumber() const { return mPadSubsetNumber; }
 
   void setValue(const size_t channel, const T& value) { mData[channel] = value; }
-  const T getValue(const size_t channel) const { return mData[channel]; }
+  const T getValue(const size_t channel) const
+  {
+    assert(channel < mData.size());
+    return mData[channel];
+  }
 
   void setValue(const size_t row, const size_t pad, const T& value);
   const T getValue(const size_t row, const size_t pad) const;

Detectors/TPC/base/include/TPCBase/CalDet.h

@@ -30,6 +30,12 @@
 #include "Rtypes.h"
 #endif
 
+#ifndef NDEBUG
+#undef NDEBUG
+// always enable assert
+#include <cassert>
+#endif
+
 namespace o2
 {
 namespace tpc
@@ -211,7 +217,26 @@ inline const T CalDet<T>::getValue(const ROC roc, const size_t row, const size_t
     }
     case PadSubset::Region: {
       const auto globalRow = roc.isOROC() ? mappedRow + mapper.getNumberOfRowsROC(ROC(0)) : mappedRow;
-      return mData[Mapper::REGION[globalRow] + roc.getSector() * Mapper::NREGIONS].getValue(Mapper::OFFSETCRUGLOBAL[globalRow] + mappedPad);
+      const auto dataRow = Mapper::REGION[globalRow] + roc.getSector() * Mapper::NREGIONS;
+      const auto index = Mapper::OFFSETCRUGLOBAL[globalRow] + mappedPad;
+      assert(dataRow < mData.size());
+      if (index >= mData[dataRow].getData().size()) {
+        // S. Wenzel: We shouldn't come here but we do. For instance for CalDet calibrations loaded from
+        // creator.loadIDCPadFlags(1731274461770);
+
+        // In this case there is an index overflow, leading to invalid reads and potentially a segfault.
+        // To increase stability, for now returning a trivial answer. This can be removed once either the algorithm
+        // or the calibration data has been fixed.
+#ifndef GPUCA_ALIGPUCODE // hide from GPU standalone compilation
+        static bool printMsg = true;
+        if (printMsg) {
+          LOG(error) << "Out of bound access in TPC CalDet ROC " << roc << " row " << row << " pad " << pad << " (no more messages printed)";
+        }
+        printMsg = false;
+#endif
+        return T{};
+      }
+      return mData[dataRow].getValue(index);
       break;
     }
   }

Detectors/TPC/base/src/TPCFlagsMemberCustomStreamer.cxx

@@ -39,6 +39,7 @@ void MemberVectorPadFlagsStreamer(TBuffer& R__b, void* objp, int n)
   }
   std::vector<o2::tpc::PadFlags>* obj = static_cast<std::vector<o2::tpc::PadFlags>*>(objp);
   if (R__b.IsReading()) {
+    obj->clear();
     std::vector<int> R__stl;
     R__stl.clear();
     int R__n;
@@ -50,7 +51,8 @@ void MemberVectorPadFlagsStreamer(TBuffer& R__b, void* objp, int n)
       R__stl.push_back(readtemp);
     }
     auto data = reinterpret_cast<unsigned short*>(R__stl.data());
-    for (int i = 0; i < R__n; ++i) {
+    constexpr size_t bloatfactor = sizeof(int) / sizeof(o2::tpc::PadFlags);
+    for (int i = 0; i < bloatfactor * R__n; ++i) {
       obj->push_back(static_cast<o2::tpc::PadFlags>(data[i]));
     }
   } else {
@@ -63,17 +65,22 @@ void MemberVectorPadFlagsStreamer(TBuffer& R__b, void* objp, int n)
 }
 
 // register the streamer via static global initialization (on library load)
+// the streamer is only correct in combination with new ROOT
+#if ROOT_VERSION_CODE >= ROOT_VERSION(6, 33, 00)
 namespace ROOT
 {
 static __attribute__((used)) int _R__dummyStreamer_3 =
   ([]() {
     auto cl = TClass::GetClass<o2::tpc::CalArray<o2::tpc::PadFlags>>();
     if (cl) {
-      cl->AdoptMemberStreamer("mData", new TMemberStreamer(MemberVectorPadFlagsStreamer));
+      if (!getenv("TPC_PADFLAGS_STREAMER_OFF")) {
+        cl->AdoptMemberStreamer("mData", new TMemberStreamer(MemberVectorPadFlagsStreamer));
+      }
     } else {
       // we should never come here ... and if we do we should assert/fail
       assert(false);
     }
     return 0;
   })();
 } // namespace ROOT
+#endif

Detectors/TPC/base/test/testTPCCalDet.cxx

@@ -24,6 +24,7 @@
 #include "TPCBase/CalDet.h"
 #include "TFile.h"
 #include "Framework/TypeTraits.h"
+#include "TPCBase/DeadChannelMapCreator.h"
 
 namespace o2::tpc
 {
@@ -344,4 +345,12 @@ BOOST_AUTO_TEST_CASE(CalDetTypeTest)
   BOOST_CHECK(testDict == true);
 }
 
+BOOST_AUTO_TEST_CASE(CalDetStreamerTest)
+{
+  // simple code executing the TPC IDCPadFlags loading in a standalone env --> easy to valgrind
+  o2::tpc::DeadChannelMapCreator creator{};
+  creator.init("https://alice-ccdb.cern.ch");
+  creator.loadIDCPadFlags(1731274461770);
+}
+
 } // namespace o2::tpc

Framework/AnalysisSupport/CMakeLists.txt

@@ -16,6 +16,12 @@ if(TARGET JAliEn::JAliEn)
   set(EXTRA_TARGETS XRootD::Client JAliEn::JAliEn)
 endif()
 
+o2_add_library(FrameworkOnDemandTablesSupport
+               SOURCES src/OnDemandPlugin.cxx
+                       src/AODReaderHelpers.cxx
+               PRIVATE_INCLUDE_DIRECTORIES ${CMAKE_CURRENT_LIST_DIR}/src
+               PUBLIC_LINK_LIBRARIES O2::Framework ${EXTRA_TARGETS})
+
 o2_add_library(FrameworkAnalysisSupport
                SOURCES src/Plugin.cxx
                        src/DataInputDirector.cxx