[QC-1176] Display a warning for developers if a registered object is …

…not mergeable (#2400) --------- Co-authored-by: Michal Tichák <[email protected]>
AliceO2Group · Nov 25, 2024 · 92fe168 · 92fe168
1 parent c0ec83f
commit 92fe168
Show file tree

Hide file tree

Showing 14 changed files with 62 additions and 36 deletions.
diff --git a/Framework/include/QualityControl/ObjectsManager.h b/Framework/include/QualityControl/ObjectsManager.h
@@ -21,12 +21,14 @@
 #include "QualityControl/Activity.h"
 #include "QualityControl/MonitorObject.h"
 #include "QualityControl/MonitorObjectCollection.h"
+#include <Mergers/Mergeable.h>
 // stl
+#include <concepts>
 #include <string>
 #include <memory>
+#include <type_traits>
 
 class TObject;
-class TObjArray;
 
 namespace o2::quality_control::core
 {
@@ -76,10 +78,26 @@ class ObjectsManager
   /**
    * Start publishing the object obj, i.e. it will be pushed forward in the workflow at regular intervals.
    * The ownership remains to the caller.
+   * @param IgnoreMergeable if you want to ignore static_assert check for Mergeable
+   * @param T type of object that we want to publish.
    * @param obj The object to publish.
    * @throws DuplicateObjectError
    */
-  void startPublishing(TObject* obj, PublicationPolicy = PublicationPolicy::Forever);
+  template <bool IgnoreMergeable = false, typename T>
+  void startPublishing(T obj, PublicationPolicy policy = PublicationPolicy::Forever)
+  {
+    // We don't want to do this compile time check in PostProcessing, and we want to turn off runtime check as well
+    bool ignoreMergeableRuntime = IgnoreMergeable;
+#ifndef QUALITYCONTROL_POSTPROCESSINTERFACE_H
+    static_assert(std::same_as<std::remove_pointer_t<T>, TObject> ||
+                    IgnoreMergeable || mergers::Mergeable<T>,
+                  "you are trying to startPublishing object that is not mergeable."
+                  " If you know what you are doing use startPublishing<true>(...)");
+#else
+    ignoreMergeableRuntime = true;
+#endif
+    startPublishingImpl(obj, policy, ignoreMergeableRuntime);
+  }
 
   /**
    * Stop publishing this object
@@ -223,6 +241,8 @@ class ObjectsManager
   bool mUpdateServiceDiscovery;
   Activity mActivity;
   std::vector<std::string> mMovingWindowsList;
+
+  void startPublishingImpl(TObject* obj, PublicationPolicy, bool ignoreMergeableWarning);
 };
 
 } // namespace o2::quality_control::core

diff --git a/Framework/src/ObjectsManager.cxx b/Framework/src/ObjectsManager.cxx
@@ -60,16 +60,22 @@ ObjectsManager::~ObjectsManager()
   ILOG(Debug, Devel) << "ObjectsManager destructor" << ENDM;
 }
 
-void ObjectsManager::startPublishing(TObject* object, PublicationPolicy publicationPolicy)
+void ObjectsManager::startPublishingImpl(TObject* object, PublicationPolicy publicationPolicy, bool ignoreMergeableWarning)
 {
   if (!object) {
     ILOG(Warning, Support) << "A nullptr provided to ObjectManager::startPublishing" << ENDM;
     return;
   }
+
   if (mMonitorObjects->FindObject(object->GetName()) != nullptr) {
     ILOG(Warning, Support) << "Object is already being published (" << object->GetName() << "), will remove it and add the new one" << ENDM;
     stopPublishing(object->GetName());
   }
+
+  if (!ignoreMergeableWarning && !mergers::isMergeable(object)) {
+    ILOG(Warning, Support) << "Object '" + std::string(object->GetName()) + "' with type '" + std::string(object->ClassName()) + "' is not one of the mergeable types, it will not be correctly merged in distributed setups, such as P2 and Grid" << ENDM;
+  }
+
   auto* newObject = new MonitorObject(object, mTaskName, mTaskClass, mDetectorName);
   newObject->setIsOwner(false);
   newObject->setActivity(mActivity);

diff --git a/Framework/src/SliceTrendingTask.cxx b/Framework/src/SliceTrendingTask.cxx
@@ -671,4 +671,4 @@ std::string SliceTrendingTask::beautifyTitle(const std::string_view rawtitle, co
   }
 
   return beautified;
-}
+}
diff --git a/Framework/test/testObjectsManager.cxx b/Framework/test/testObjectsManager.cxx
@@ -53,12 +53,12 @@ BOOST_AUTO_TEST_CASE(duplicate_object_test)
   config.consulUrl = "";
   ObjectsManager objectsManager(config.taskName, config.taskClass, config.detectorName, config.consulUrl, 0, true);
   TObjString s("content");
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
-  BOOST_CHECK_NO_THROW(objectsManager.startPublishing(&s, PublicationPolicy::Forever));
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
+  BOOST_CHECK_NO_THROW(objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever));
   BOOST_REQUIRE(objectsManager.getMonitorObject("content") != nullptr);
 
   TObjString s2("content");
-  BOOST_CHECK_NO_THROW(objectsManager.startPublishing(&s2, PublicationPolicy::Forever));
+  BOOST_CHECK_NO_THROW(objectsManager.startPublishing<true>(&s2, PublicationPolicy::Forever));
   auto mo2 = objectsManager.getMonitorObject("content");
   BOOST_REQUIRE(mo2 != nullptr);
   BOOST_REQUIRE(mo2->getObject() != &s);
@@ -73,8 +73,8 @@ BOOST_AUTO_TEST_CASE(is_being_published_test)
   ObjectsManager objectsManager(config.taskName, config.taskClass, config.detectorName, config.consulUrl, 0, true);
   TObjString s("content");
   BOOST_CHECK(!objectsManager.isBeingPublished("content"));
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
-  BOOST_CHECK_NO_THROW(objectsManager.startPublishing(&s, PublicationPolicy::Forever));
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
+  BOOST_CHECK_NO_THROW(objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever));
   BOOST_CHECK(objectsManager.isBeingPublished("content"));
 }
 
@@ -84,46 +84,46 @@ BOOST_AUTO_TEST_CASE(unpublish_test)
   config.taskName = "test";
   ObjectsManager objectsManager(config.taskName, config.taskClass, config.detectorName, config.consulUrl, 0, true);
   TObjString s("content");
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 1);
   objectsManager.stopPublishing(&s);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 1);
   objectsManager.stopPublishing("content");
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
   BOOST_CHECK_THROW(objectsManager.stopPublishing("content"), ObjectNotFoundError);
   BOOST_CHECK_THROW(objectsManager.stopPublishing("asdf"), ObjectNotFoundError);
 
   // unpublish all
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 1);
   objectsManager.stopPublishingAll();
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
   BOOST_CHECK_NO_THROW(objectsManager.stopPublishingAll());
 
   // unpublish after deletion
   auto s2 = new TObjString("content");
-  objectsManager.startPublishing(s2, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(s2, PublicationPolicy::Forever);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 1);
   delete s2;
   objectsManager.stopPublishing(s2);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
 
   // unpublish for publication policy
   auto s3 = new TObjString("content3");
-  objectsManager.startPublishing(s3, PublicationPolicy::Once);
+  objectsManager.startPublishing<true>(s3, PublicationPolicy::Once);
   auto s4 = new TObjString("content4");
-  objectsManager.startPublishing(s4, PublicationPolicy::Once);
+  objectsManager.startPublishing<true>(s4, PublicationPolicy::Once);
   auto s5 = new TObjString("content5");
-  objectsManager.startPublishing(s5, PublicationPolicy::ThroughStop);
+  objectsManager.startPublishing<true>(s5, PublicationPolicy::ThroughStop);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 3);
   objectsManager.stopPublishing(PublicationPolicy::Once);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 1);
   objectsManager.stopPublishing(PublicationPolicy::ThroughStop);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
 
-  objectsManager.startPublishing(s3, PublicationPolicy::Once);
+  objectsManager.startPublishing<true>(s3, PublicationPolicy::Once);
   objectsManager.stopPublishing(s3);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
   BOOST_CHECK_NO_THROW(objectsManager.stopPublishing(PublicationPolicy::Once));
@@ -145,8 +145,8 @@ BOOST_AUTO_TEST_CASE(getters_test)
   TObjString s("content");
   TH1F h("histo", "h", 100, 0, 99);
 
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
-  objectsManager.startPublishing(&h, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&h, PublicationPolicy::Forever);
 
   // basic gets
   BOOST_CHECK_NO_THROW(objectsManager.getMonitorObject("content"));
@@ -174,8 +174,8 @@ BOOST_AUTO_TEST_CASE(metadata_test)
 
   TObjString s("content");
   TH1F h("histo", "h", 100, 0, 99);
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
-  objectsManager.startPublishing(&h, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&h, PublicationPolicy::Forever);
 
   objectsManager.addMetadata("content", "aaa", "bbb");
   BOOST_CHECK_EQUAL(objectsManager.getMonitorObject("content")->getMetadataMap().at("aaa"), "bbb");
@@ -211,7 +211,7 @@ BOOST_AUTO_TEST_CASE(feed_with_nullptr)
   config.consulUrl = "";
   ObjectsManager objectsManager(config.taskName, config.taskClass, config.detectorName, config.consulUrl, 0, true);
 
-  BOOST_CHECK_NO_THROW(objectsManager.startPublishing(nullptr, PublicationPolicy::Forever));
+  BOOST_CHECK_NO_THROW(objectsManager.startPublishing<true>(nullptr, PublicationPolicy::Forever));
   BOOST_CHECK_NO_THROW(objectsManager.setDefaultDrawOptions(nullptr, ""));
   BOOST_CHECK_NO_THROW(objectsManager.setDisplayHint(nullptr, ""));
   BOOST_CHECK_NO_THROW(objectsManager.stopPublishing(nullptr));

diff --git a/Framework/test/testPublisher.cxx b/Framework/test/testPublisher.cxx
@@ -38,7 +38,7 @@ BOOST_AUTO_TEST_CASE(publisher_test)
   std::string consulUrl = "invalid";
   ObjectsManager objectsManager(taskName, "taskClass", detectorName, consulUrl, 0, true);
   TObjString s("content");
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
 
   TObjString* s2 = (TObjString*)(objectsManager.getMonitorObject("content")->getObject());
   BOOST_CHECK_EQUAL(s.GetString(), s2->GetString());

diff --git a/Modules/CTP/src/CountersQcTask.cxx b/Modules/CTP/src/CountersQcTask.cxx
@@ -62,7 +62,7 @@ void CTPCountersTask::initialize(o2::framework::InitContext& /*ctx*/)
       mHistInputRate[i]->Draw();
       mHistInputRate[i]->SetBit(TObject::kCanDelete);
     }
-    getObjectsManager()->startPublishing(mTCanvasInputs);
+    getObjectsManager()->startPublishing<true>(mTCanvasInputs);
   }
 
   {
@@ -79,7 +79,7 @@ void CTPCountersTask::initialize(o2::framework::InitContext& /*ctx*/)
       mHistClassRate[i]->Draw();
       mHistClassRate[i]->SetBit(TObject::kCanDelete);
     }
-    getObjectsManager()->startPublishing(mTCanvasClasses);
+    getObjectsManager()->startPublishing<true>(mTCanvasClasses);
   }
 
   {
@@ -100,7 +100,7 @@ void CTPCountersTask::initialize(o2::framework::InitContext& /*ctx*/)
         mHistClassRate[k]->Draw();
         mHistClassRate[k]->SetBit(TObject::kCanDelete);
       }*/
-      getObjectsManager()->startPublishing(mTCanvasClassRates[j]);
+      getObjectsManager()->startPublishing<true>(mTCanvasClassRates[j]);
     }
   }
 
@@ -136,7 +136,7 @@ void CTPCountersTask::initialize(o2::framework::InitContext& /*ctx*/)
       mHistClassTotalCounts[i]->Draw();
       mHistClassTotalCounts[i]->SetBit(TObject::kCanDelete);
     }
-    getObjectsManager()->startPublishing(mTCanvasTotalCountsClasses);
+    getObjectsManager()->startPublishing<true>(mTCanvasTotalCountsClasses);
   }
 }
 

diff --git a/Modules/Example/src/EveryObject.cxx b/Modules/Example/src/EveryObject.cxx
@@ -82,7 +82,7 @@ void EveryObject::initialize(o2::framework::InitContext& /*ctx*/)
       mTCanvasMembers[i]->Draw();
       mTCanvasMembers[i]->SetBit(TObject::kCanDelete);
     }
-    getObjectsManager()->startPublishing(mTCanvas, PublicationPolicy::Forever);
+    getObjectsManager()->startPublishing<true>(mTCanvas, PublicationPolicy::Forever);
   }
 }
 

diff --git a/Modules/GLO/src/DataCompressionQcTask.cxx b/Modules/GLO/src/DataCompressionQcTask.cxx
@@ -70,8 +70,8 @@ void DataCompressionQcTask::initialize(o2::framework::InitContext&)
     mEntropyCompressionCanvas->DivideSquare(mCompressionHists.size());
     mCompressionCanvas->DivideSquare(mCompressionHists.size());
 
-    getObjectsManager()->startPublishing(mEntropyCompressionCanvas.get());
-    getObjectsManager()->startPublishing(mCompressionCanvas.get());
+    getObjectsManager()->startPublishing<true>(mEntropyCompressionCanvas.get());
+    getObjectsManager()->startPublishing<true>(mCompressionCanvas.get());
   }
 }
 

diff --git a/Modules/HMPID/src/HmpidTask.cxx b/Modules/HMPID/src/HmpidTask.cxx
@@ -194,7 +194,7 @@ void HmpidTask::initialize(o2::framework::InitContext& /*ctx*/)
 
   // Error messages
   CheckerMessages = new TCanvas("CheckerMessages");
-  getObjectsManager()->startPublishing(CheckerMessages);
+  getObjectsManager()->startPublishing<true>(CheckerMessages);
 
   // TH2 to check HV
   hCheckHV = new TH2F("hCheckHV", "hCheckHV", 42, -0.5, 41.5, 4, 0, 4);

diff --git a/Modules/MUON/MCH/src/PedestalsTask.cxx b/Modules/MUON/MCH/src/PedestalsTask.cxx
@@ -168,7 +168,7 @@ void PedestalsTask::initialize(o2::framework::InitContext& /*ctx*/)
   }
 
   mCanvasCheckerMessages = std::make_unique<TCanvas>("CheckerMessages", "Checker Messages", 800, 600);
-  getObjectsManager()->startPublishing(mCanvasCheckerMessages.get());
+  getObjectsManager()->startPublishing<true>(mCanvasCheckerMessages.get());
 
   mPrintLevel = 0;
 }

diff --git a/Modules/TPC/src/Clusters.cxx b/Modules/TPC/src/Clusters.cxx
@@ -82,7 +82,7 @@ void Clusters::initialize(InitContext& /*ctx*/)
     addAndPublish(getObjectsManager(), mTimeBinCanvasVec, { "c_Sides_Time_Bin", "c_ROCs_Time_Bin_1D", "c_ROCs_Time_Bin_2D" });
 
     for (auto& wrapper : mWrapperVector) {
-      getObjectsManager()->startPublishing(&wrapper);
+      getObjectsManager()->startPublishing<true>(&wrapper);
     }
   }
 }

diff --git a/Modules/TPC/src/JunkDetection.cxx b/Modules/TPC/src/JunkDetection.cxx
@@ -45,7 +45,7 @@ void JunkDetection::initialize(o2::framework::InitContext&)
   mJDHistos.emplace_back(new TH2F("h_removed_Strategy_B", "Removed Strategy (B)", 1, 0, 1, 1, 0, 1)); // dummy for the objectsManager
 
   if (!mIsMergeable) {
-    getObjectsManager()->startPublishing(mJDCanv.get());
+    getObjectsManager()->startPublishing<true>(mJDCanv.get());
   }
   for (const auto& hist : mJDHistos) {
     getObjectsManager()->startPublishing(hist);

diff --git a/Modules/TPC/src/RawDigits.cxx b/Modules/TPC/src/RawDigits.cxx
@@ -73,7 +73,7 @@ void RawDigits::initialize(o2::framework::InitContext& /*ctx*/)
     addAndPublish(getObjectsManager(), mTimeBinCanvasVec, { "c_Sides_Time_Bin", "c_ROCs_Time_Bin_1D", "c_ROCs_Time_Bin_2D" });
 
     for (auto& wrapper : mWrapperVector) {
-      getObjectsManager()->startPublishing(&wrapper);
+      getObjectsManager()->startPublishing<true>(&wrapper);
     }
   }
 

diff --git a/Modules/TPC/src/Utility.cxx b/Modules/TPC/src/Utility.cxx
@@ -60,7 +60,7 @@ void addAndPublish(std::shared_ptr<o2::quality_control::core::ObjectsManager> ob
   for (const auto& canvName : canvNames) {
     canVec.emplace_back(std::make_unique<TCanvas>(canvName.data()));
     auto canvas = canVec.back().get();
-    objectsManager->startPublishing(canvas);
+    objectsManager->startPublishing<true>(canvas);
     if (metaData.size() != 0) {
       for (const auto& [key, value] : metaData) {
         objectsManager->addMetadata(canvas->GetName(), key, value);
-Original file line number
+Diff line change
@@ Expand Up @@
       }
       return beautified;
-    }
+    }