From 6fb904ada0f6e2c93785af661b7fd7663e10d54e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Tich=C3=A1k?= Date: Mon, 30 Sep 2024 15:02:45 +0200 Subject: [PATCH] switch MOs in MonitorObjectCollection when run number is higher on new tests --- Framework/src/MonitorObjectCollection.cxx | 20 ++++- .../test/testMonitorObjectCollection.cxx | 75 ++++++++++++++++++- 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/Framework/src/MonitorObjectCollection.cxx b/Framework/src/MonitorObjectCollection.cxx index 31b021289f..abfc4572be 100644 --- a/Framework/src/MonitorObjectCollection.cxx +++ b/Framework/src/MonitorObjectCollection.cxx @@ -46,16 +46,32 @@ void MonitorObjectCollection::merge(mergers::MergeInterface* const other) if (!otherMO || !targetMO) { throw std::runtime_error("The target object or the other object could not be casted to MonitorObject."); } - if (!reportedMismatchingRunNumbers && targetMO->getActivity().mId != otherMO->getActivity().mId) { + + if (otherMO->getActivity().mId > targetMO->getActivity().mId) { + ILOG(Error, Ops) << "The run number of the input object '" << otherMO->GetName() << "' (" + << otherMO->getActivity().mId << ") " + << "is higher than the one of the target object '" + << targetMO->GetName() << "' (" << targetMO->getActivity().mId + << "). Replacing the merged object with input, " + << "but THIS SHOULD BE IMMEDIATELY ADDRESSED IN PRODUCTION. " + << "QC objects from other setups are reaching this one." + << ENDM; + otherMO->Copy(*targetMO); + continue; + } + + if (!reportedMismatchingRunNumbers && otherMO->getActivity().mId < targetMO->getActivity().mId) { ILOG(Error, Ops) << "The run number of the input object '" << otherMO->GetName() << "' (" << otherMO->getActivity().mId << ") " << "does not match the run number of the target object '" << targetMO->GetName() << "' (" << targetMO->getActivity().mId - << "). Trying to continue, but THIS SHOULD BE IMMEDIATELY ADDRESSED IN PRODUCTION. " + << "). Ignoring this object and trying to continue, but THIS SHOULD BE IMMEDIATELY ADDRESSED IN PRODUCTION. " << "QC objects from other setups are reaching this one. Will not report more mismatches in this collection." << ENDM; reportedMismatchingRunNumbers = true; + continue; } + // That might be another collection or a concrete object to be merged, we walk on the collection recursively. algorithm::merge(targetMO->getObject(), otherMO->getObject()); if (otherMO->getValidity().isValid()) { diff --git a/Framework/test/testMonitorObjectCollection.cxx b/Framework/test/testMonitorObjectCollection.cxx index 2c38208229..b097056eb5 100644 --- a/Framework/test/testMonitorObjectCollection.cxx +++ b/Framework/test/testMonitorObjectCollection.cxx @@ -16,8 +16,8 @@ #include "QualityControl/MonitorObjectCollection.h" #include "QualityControl/MonitorObject.h" -#include "QualityControl/QcInfoLogger.h" +#include #include #include #include @@ -96,6 +96,77 @@ TEST_CASE("monitor_object_collection_merge") delete target; } +TEST_CASE("monitor_object_collection_merge_different_id") +{ + const auto toHisto = [](std::unique_ptr& collection) -> TH1I* { + return dynamic_cast(dynamic_cast(collection->At(0))->getObject()); + }; + + constexpr size_t bins = 10; + constexpr size_t min = 0; + constexpr size_t max = 10; + + SECTION("other has higher run number than target") + { + auto target = std::make_unique(); + + auto* targetTH1I = new TH1I("histo 1d", "original", bins, min, max); + targetTH1I->Fill(5); + auto* targetMoTH1I = new MonitorObject(targetTH1I, "histo 1d", "class", "DET"); + targetMoTH1I->setActivity({ 123, "PHYSICS", "LHC32x", "apass2", "qc_async", { 10, 20 } }); + targetMoTH1I->setIsOwner(true); + target->Add(targetMoTH1I); + + auto other = std::make_unique(); + other->SetOwner(true); + + auto* otherTH1I = new TH1I("histo 1d", "input", bins, min, max); + otherTH1I->Fill(2); + auto* otherMoTH1I = new MonitorObject(otherTH1I, "histo 1d", "class", "DET"); + otherMoTH1I->setActivity({ 1234, "PHYSICS", "LHC32x", "apass2", "qc_async", { 43, 60 } }); + otherMoTH1I->setIsOwner(true); + other->Add(otherMoTH1I); + + CHECK_NOTHROW(algorithm::merge(target.get(), other.get())); + auto* h1orig = toHisto(target); + auto* h1other = toHisto(other); + REQUIRE(h1orig->GetAt(3) == 1); + for (size_t i = 0; i != h1orig->GetSize(); ++i) { + REQUIRE(h1orig->GetAt(i) == h1other->GetAt(i)); + } + } + + SECTION("other has lower run number than target") + { + auto target = std::make_unique(); + + auto* targetTH1I = new TH1I("histo 1d", "original", bins, min, max); + targetTH1I->Fill(5); + auto* targetMoTH1I = new MonitorObject(targetTH1I, "histo 1d", "class", "DET"); + targetMoTH1I->setActivity({ 1234, "PHYSICS", "LHC32x", "apass2", "qc_async", { 10, 20 } }); + targetMoTH1I->setIsOwner(true); + target->Add(targetMoTH1I); + + auto other = std::make_unique(); + other->SetOwner(true); + + auto* otherTH1I = new TH1I("histo 1d", "input", bins, min, max); + otherTH1I->Fill(2); + auto* otherMoTH1I = new MonitorObject(otherTH1I, "histo 1d", "class", "DET"); + otherMoTH1I->setActivity({ 123, "PHYSICS", "LHC32x", "apass2", "qc_async", { 43, 60 } }); + otherMoTH1I->setIsOwner(true); + other->Add(otherMoTH1I); + + CHECK_NOTHROW(algorithm::merge(target.get(), other.get())); + auto* h1orig = toHisto(target); + auto* h1other = toHisto(other); + REQUIRE(h1orig->At(h1orig->FindBin(5)) == 1); + REQUIRE(h1other->At(h1other->FindBin(5)) == 0); + REQUIRE(h1orig->At(h1orig->FindBin(2)) == 0); + REQUIRE(h1other->At(h1other->FindBin(2)) == 1); + } +} + TEST_CASE("monitor_object_collection_post_deserialization") { const size_t bins = 10; @@ -171,4 +242,4 @@ TEST_CASE("monitor_object_collection_clone_mw") delete mwMOC2; } -} // namespace o2::quality_control::core \ No newline at end of file +} // namespace o2::quality_control::core