Bugfix: clear the output containers for the TPC HwClusterer consistently

Have to clear the output containers inside the process method as not only the
containers are cleared but also the cluster counter. Clearing the containers
externally leaves the cluster counter unchanged and leads to an inconsistency
between cluster container and MC label container (the latter just grows with
every call). This is probably a bug of the TPC HwClusterer.

Also implementing:
- some minor changes on the logging output.
- boundary check for MC label object processing in the decoder

Author: matthiasrichter

Detectors/TPC/workflow/src/ClusterConverterSpec.cxx

@@ -88,15 +88,17 @@ DataProcessorSpec getClusterConverterSpec(bool sendMC, int fanNumber)
       // this will return a span of TPC clusters
       auto inClusters = pc.inputs().get<std::vector<o2::TPC::Cluster>>("clusterin");
       int nClusters = inClusters.size();
-      LOG(INFO) << "got clusters from input: " << nClusters;
 
       // MC labels are received as one container of labels in the sequence matching clusters
       // in the input
       std::unique_ptr<const MCLabelContainer> mcin;
       MCLabelContainer mcout;
+      std::string mcMesssage;
       if (sendMC) {
         mcin = std::move(pc.inputs().get<MCLabelContainer*>("mclblin"));
+        mcMesssage = ", " + std::to_string(mcin->getIndexedSize()) + " MC label objects";
       }
+      LOG(INFO) << "got " << nClusters << " cluster(s) from input" << mcMesssage;
 
       // clusters need to be sorted to write clusters of one CRU to raw pages
       struct ClusterMapper {

Detectors/TPC/workflow/src/ClusterDecoderRawSpec.cxx

@@ -28,6 +28,7 @@
 #include <cassert>
 #include <iomanip>
 #include <string>
+#include <numeric>
 
 using namespace o2::framework;
 using namespace o2::header;
@@ -123,6 +124,7 @@ DataProcessorSpec getClusterDecoderRawSpec(bool sendMC, int fanNumber)
       // only for single 8kb pages. So we have to add the raw pages individually and create
       // MC label containers for the corresponding clusters.
       size_t mcinPos = 0;
+      size_t totalNumberOfClusters = 0;
       for (size_t page = 0; page < nPages; page++) {
         inputList.emplace_back(reinterpret_cast<const ClusterHardwareContainer*>(ref.payload + page * 8192), 1);
         const ClusterHardwareContainer& container = *(inputList.back().first);
@@ -131,15 +133,24 @@ DataProcessorSpec getClusterDecoderRawSpec(bool sendMC, int fanNumber)
                     << "CRU " << std::setw(3) << container.CRU << " "               //
                     << std::setw(3) << container.numberOfClusters << " cluster(s)"; //
         }
+        totalNumberOfClusters += container.numberOfClusters;
         if (mcin) {
-          for (size_t mccopyPos = 0; mccopyPos < container.numberOfClusters; mccopyPos++, mcinPos++) {
+          for (size_t mccopyPos = 0;
+               mccopyPos < container.numberOfClusters && mcinPos < mcin->getIndexedSize();
+               mccopyPos++, mcinPos++) {
             for (auto const& label : mcin->getLabels(mcinPos)) {
               mcinCopies[page].addElement(mccopyPos, label);
             }
           }
         }
       }
-      assert(!mcin || mcinPos == mcin->getIndexedSize());
+      // FIXME: introduce error handling policy: throw, ignore, warn
+      //assert(!mcin || mcinPos == mcin->getIndexedSize());
+      if (mcin && mcinPos != totalNumberOfClusters) {
+        LOG(ERROR) << "inconsistent number of MC label objects processed"
+                   << ", expecting MC label objects for " << totalNumberOfClusters << " cluster(s)"
+                   << ", got " << mcin->getIndexedSize();
+      }
       // output of the decoder is sorted in (sector,globalPadRow) coordinates, individual
       // containers are created for clusters and MC labels per (sector,globalPadRow) address
       std::vector<ClusterNativeContainer> cont;
@@ -160,7 +171,7 @@ DataProcessorSpec getClusterDecoderRawSpec(bool sendMC, int fanNumber)
       auto* target = pc.outputs().newChunk(OutputRef{ "clusterout", fanSpec, std::move(rawHeaderStack) }, totalSize).data;
 
       for (const auto& coll : cont) {
-        if (verbosity > 0) {
+        if (verbosity > 1) {
           LOG(INFO) << "decoder " << std::setw(2) << sectorHeader->sector                             //
                     << ": decoded " << std::setw(4) << coll.clusters.size() << " clusters on sector " //
                     << std::setw(2) << (int)coll.sector << "[" << (int)coll.globalPadRow << "]";      //
@@ -173,7 +184,9 @@ DataProcessorSpec getClusterDecoderRawSpec(bool sendMC, int fanNumber)
       }
       if (sendMC) {
         if (verbosity > 0) {
-          LOG(INFO) << "sending " << mcoutList.size() << " MC label containers" << std::endl;
+          LOG(INFO) << "sending " << mcoutList.size() << " MC label container(s) with in total "
+                    << std::accumulate(mcoutList.begin(), mcoutList.end(), size_t(0), [](size_t l, auto const& r) { return l + r.getIndexedSize(); })
+                    << " label object(s)" << std::endl;
         }
         // serialize the complete list of MC label containers
         pc.outputs().snapshot(OutputRef{ "mclblout", fanSpec, std::move(mcHeaderStack) }, mcoutList);

Detectors/TPC/workflow/src/ClustererSpec.cxx

@@ -90,7 +90,8 @@ DataProcessorSpec getClustererSpec(bool sendMC, int fanNumber)
       auto inMCLabels = pc.inputs().get<const MCLabelContainer*>("mclabels");
       auto inDigits = pc.inputs().get<const std::vector<o2::TPC::Digit>>("digits");
       if (verbosity > 0) {
-        LOG(INFO) << "received " << inDigits.size() << " digits";
+        LOG(INFO) << "received " << inDigits.size() << " digits, "
+                  << inMCLabels->getIndexedSize() << " MC label objects";
       }
       if (!clusterers[sector]) {
         // create the clusterer for this sector, take the same target arrays for all clusterers
@@ -103,13 +104,19 @@ DataProcessorSpec getClustererSpec(bool sendMC, int fanNumber)
       if (verbosity > 0) {
         LOG(INFO) << "processing " << inDigits.size() << " digit object(s) of sector " << sectorHeader->sector;
       }
-      clusterArray.clear(); // this would also be done in the HwClusterer if the clearContainerFirst of process() would be set to true instead of false
-      mctruthArray.clear(); // this would also be done in the HwClusterer if the clearContainerFirst of process() would be set to true instead of false
-      clusterer->process(inDigits, inMCLabels.get(), false);
+      // process the digits and MC labels, the bool parameter controls whether to clear all
+      // internal data or not. Have to clear it inside the process method as not only the containers
+      // are cleared but also the cluster counter. Clearing the containers externally leaves the
+      // cluster counter unchanged and leads to an inconsistency between cluster container and
+      // MC label container (the latter just grows with every call).
+      clusterer->process(inDigits, inMCLabels.get(), true /* clear output containers and cluster counter */);
       const std::vector<o2::TPC::Digit> emptyDigits;
       clusterer->finishProcess(emptyDigits, nullptr, false); // keep here the false, otherwise the clusters are lost of they are not stored in the meantime
       if (verbosity > 0) {
-        LOG(INFO) << "clusterer produced " << clusterArray.size() << " cluster container";
+        LOG(INFO) << "clusterer produced " << clusterArray.size() << " cluster(s)";
+        if (sendMC) {
+          LOG(INFO) << "clusterer produced " << mctruthArray.getIndexedSize() << " MC label object(s)";
+        }
       }
       pc.outputs().snapshot(OutputRef{ "clusters", fanSpec, { *sectorHeader } }, clusterArray);
       if (sendMC) {