Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[QC-1201] Replace the merged object with the input if it has higher run number #2443

Merged
merged 1 commit into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions Framework/src/MonitorObjectCollection.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
justonedev1 marked this conversation as resolved.
Show resolved Hide resolved
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);
justonedev1 marked this conversation as resolved.
Show resolved Hide resolved
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()) {
Expand Down
75 changes: 73 additions & 2 deletions Framework/test/testMonitorObjectCollection.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

#include "QualityControl/MonitorObjectCollection.h"
#include "QualityControl/MonitorObject.h"
#include "QualityControl/QcInfoLogger.h"

#include <TH1.h>
#include <TH1I.h>
#include <TH2I.h>
#include <TH2I.h>
Expand Down Expand Up @@ -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<MonitorObjectCollection>& collection) -> TH1I* {
return dynamic_cast<TH1I*>(dynamic_cast<MonitorObject*>(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<MonitorObjectCollection>();

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<MonitorObjectCollection>();
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<MonitorObjectCollection>();

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<MonitorObjectCollection>();
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;
Expand Down Expand Up @@ -171,4 +242,4 @@ TEST_CASE("monitor_object_collection_clone_mw")
delete mwMOC2;
}

} // namespace o2::quality_control::core
} // namespace o2::quality_control::core
Loading