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-885] Modify reference check to also accept a TH1 … #2358

Merged
merged 26 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1a6b31d
[QC-885] Modify ReferenceComparatorCheck to also accept a TH1 instead…
Barthelemy Jun 26, 2024
603a00c
format
Barthelemy Jun 26, 2024
626e7c4
add beautify and doc
Barthelemy Jun 26, 2024
d23cfcd
Format
Barthelemy Jun 26, 2024
714a486
Format
Barthelemy Jun 26, 2024
466a984
cout
Barthelemy Jun 26, 2024
a1ad044
Update ReferenceComparatorCheck.cxx
Barthelemy Jun 26, 2024
4755f09
Update Advanced.md
Barthelemy Jun 26, 2024
d6cb6e1
Update Advanced.md
Barthelemy Jun 26, 2024
6f9e9ef
Update DevelopersTips.md
Barthelemy Jun 26, 2024
4b1c4d4
Merge branch 'master' into ref-plot-check
Barthelemy Jul 1, 2024
142565f
Update Framework/include/QualityControl/ReferenceUtils.h
Barthelemy Jul 1, 2024
71e7460
Update Modules/Common/src/ReferenceComparatorCheck.cxx
Barthelemy Jul 1, 2024
591aa86
Update doc/Advanced.md
Barthelemy Jul 1, 2024
5523798
Update doc/Advanced.md
Barthelemy Jul 1, 2024
81a00d1
Update ReferenceComparatorCheck.cxx
Barthelemy Jul 1, 2024
5262cce
Update ReferenceComparatorCheck.cxx
Barthelemy Jul 1, 2024
ee37571
- use int for the run
Barthelemy Jul 1, 2024
3c62749
format
Barthelemy Jul 1, 2024
da08db6
return a tuple
Barthelemy Jul 1, 2024
1ffb5ed
format
Barthelemy Jul 2, 2024
f84803a
return null quality if no reference run number is provided.
Barthelemy Jul 3, 2024
3da5995
extraction
Barthelemy Jul 3, 2024
da17738
- Keep the activity parameter but document it
Barthelemy Jul 5, 2024
371a1a6
Use extended parameters.
Barthelemy Jul 5, 2024
fd93cdf
pass only 1 activity for reference and not the reference run + the cu…
Barthelemy Jul 5, 2024
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
7 changes: 7 additions & 0 deletions Framework/include/QualityControl/Check.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
// O2
#include <Framework/DataProcessorSpec.h>
// QC
#include "QualityControl/DatabaseInterface.h"
#include "QualityControl/QualityObject.h"
#include "QualityControl/CheckConfig.h"
#include "QualityControl/CheckInterface.h"

namespace o2::quality_control::core
{
Expand Down Expand Up @@ -92,6 +94,11 @@ class Check
static CheckConfig extractConfig(const core::CommonSpec&, const CheckSpec&);
static framework::OutputSpec createOutputSpec(const std::string& detector, const std::string& checkName);

void setDatabase(std::shared_ptr<o2::quality_control::repository::DatabaseInterface> database)
{
mCheckInterface->setDatabase(database);
}

private:
void beautify(std::map<std::string, std::shared_ptr<core::MonitorObject>>& moMap, const core::Quality& quality);

Expand Down
13 changes: 13 additions & 0 deletions Framework/include/QualityControl/CheckInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#ifndef QC_CHECKER_CHECKINTERFACE_H
#define QC_CHECKER_CHECKINTERFACE_H

#include "QualityControl/DatabaseInterface.h"
#include "QualityControl/Quality.h"
#include "QualityControl/UserCodeInterface.h"
#include "QualityControl/Activity.h"
Expand Down Expand Up @@ -82,10 +83,22 @@ class CheckInterface : public core::UserCodeInterface
virtual void startOfActivity(const core::Activity& activity); // not fully abstract because we don't want to change all the existing subclasses
virtual void endOfActivity(const core::Activity& activity); // not fully abstract because we don't want to change all the existing subclasses

void setDatabase(std::shared_ptr<o2::quality_control::repository::DatabaseInterface> database)
{
mDatabase = database;
}

protected:
/// \brief Called each time mCustomParameters is updated.
virtual void configure() override;

/// \brief Retrieve a reference plot at the provided path, matching the give activity and for the provided run.
/// the activity is the current one, while the run number is the reference run.
std::shared_ptr<MonitorObject> retrieveReference(std::string path, int referenceRun, Activity activity);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is only one correct/expected Activity that the user shall provide and the framework has the knowledge of it, why even ask the user to do it? Shouldn't the framework handle it?

Also, since the run numbers are unique, why even constrain the selection with the Activity? I don't understand what's the benefit, except that someone would want to fail if the reference run is from an earlier period than the current one. However, for such cases we should anyway produce more specific errors than could not find object, so we should handle it explicitly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aferrero2707 I picked these parameters based on your code. Do you have an opinion?
I would tend to agree with Piotr.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Barthelemy @knopers8 indeed, I also agree. I would say that we need two parameters from the activity:

  • the provenance, as we do not want to mix online and async plots
  • the pass name, but that's only relevant for async, where the mechanism of reference runs might be a bit more difficult to put in place

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussing with Andrea as well, I came to the conclusion that we should keep the parameter activity, at least for the time being. Replacing it with provenance and pass opens the door to having more and more parameters that will match what the Activity contains in the end. Moreover, we cannot let the framework feed the activity because the CheckInterface does not store the activity. We could modify all the *Interface to start storing the activity but I believe that it would go beyond the scope of this PR.

What I did was to document what the activity parameter is.


private:
std::shared_ptr<o2::quality_control::repository::DatabaseInterface> mDatabase;

ClassDef(CheckInterface, 6)
};

Expand Down
58 changes: 58 additions & 0 deletions Framework/include/QualityControl/ReferenceUtils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright 2019-2020 CERN and copyright holders of ALICE O2.
// See https://alice-o2.web.cern.ch/copyright for details of the copyright holders.
// All rights not expressly granted are reserved.
//
// This software is distributed under the terms of the GNU General Public
// License v3 (GPL Version 3), copied verbatim in the file "COPYING".
//
// In applying this license CERN does not waive the privileges and immunities
// granted to it by virtue of its status as an Intergovernmental Organization
// or submit itself to any jurisdiction.

///
/// \file ReferenceComparatorUtils.h
/// \author Andrea Ferrero and Barthelemy von Haller
///

#ifndef QUALITYCONTROL_ReferenceComparatorUtils_H
#define QUALITYCONTROL_ReferenceComparatorUtils_H

#include <memory>
#include "QualityControl/MonitorObject.h"
#include "QualityControl/DatabaseInterface.h"
#include "QualityControl/Activity.h"
#include "QualityControl/ActivityHelpers.h"
#include "QualityControl/QcInfoLogger.h"
#include "QualityControl/RepoPathUtils.h"

namespace o2::quality_control_modules::common
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved
{

//_________________________________________________________________________________________
//
// Get the reference plot for a given MonitorObject path

static std::shared_ptr<quality_control::core::MonitorObject> getReferencePlot(quality_control::repository::DatabaseInterface* qcdb, std::string& fullPath, uint32_t referenceRun, quality_control::core::Activity activity)
{
uint64_t timeStamp = 0;
activity.mId = referenceRun;
const auto filterMetadata = quality_control::core::activity_helpers::asDatabaseMetadata(activity, false);
const auto objectValidity = qcdb->getLatestObjectValidity(activity.mProvenance + "/" + fullPath, filterMetadata);
if (objectValidity.isValid()) {
timeStamp = objectValidity.getMax() - 1;
} else {
ILOG(Warning, Devel) << "Could not find the object '" << fullPath << "' for run " << activity.mId << ENDM;
return nullptr;
}

std::string path;
std::string name;
if (!o2::quality_control::core::RepoPathUtils::splitObjectPath(fullPath, path, name)) {
return nullptr;
}
return qcdb->retrieveMO(path, name, timeStamp, activity);
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved
}

} // namespace o2::quality_control_modules::common

#endif // QUALITYCONTROL_ReferenceComparatorUtils_H
13 changes: 13 additions & 0 deletions Framework/include/QualityControl/RepoPathUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,19 @@ class RepoPathUtils

static constexpr auto allowedProvenancesMessage = R"(Allowed provenances are "qc" (real data processed synchronously), "qc_async" (real data processed asynchronously) and "qc_mc" (simulated data).)";
static bool isProvenanceAllowed(const std::string& provenance);

static bool splitObjectPath(const std::string& fullPath, std::string& path, std::string& name)
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved
{
std::string delimiter = "/";
std::string det;
size_t pos = fullPath.rfind(delimiter);
if (pos == std::string::npos) {
return false;
}
path = fullPath.substr(0, pos);
name = fullPath.substr(pos + 1);
return true;
}
};
} // namespace o2::quality_control::core

Expand Down
6 changes: 6 additions & 0 deletions Framework/src/CheckInterface.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
///

#include "QualityControl/CheckInterface.h"
#include "QualityControl/ReferenceUtils.h"
#include "QualityControl/MonitorObject.h"

#include <TClass.h>
Expand Down Expand Up @@ -59,4 +60,9 @@ void CheckInterface::endOfActivity(const Activity& activity)
// noop, override it if you want.
}

shared_ptr<MonitorObject> CheckInterface::retrieveReference(std::string path, int referenceRun, Activity activity)
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved
{
return quality_control_modules::common::getReferencePlot(mDatabase.get(), path, referenceRun, activity);
}

} // namespace o2::quality_control::checker
1 change: 1 addition & 0 deletions Framework/src/CheckRunner.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ void CheckRunner::init(framework::InitContext& iCtx)
updatePolicyManager.reset();
for (auto& [checkName, check] : mChecks) {
check.init();
check.setDatabase(mDatabase);
updatePolicyManager.addPolicy(check.getName(), check.getUpdatePolicyType(), check.getObjectsNames(), check.getAllObjectsOption(), false);
}
} catch (...) {
Expand Down
3 changes: 3 additions & 0 deletions Modules/Common/include/Common/ReferenceComparatorCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,12 @@ class ReferenceComparatorCheck : public o2::quality_control::checker::CheckInter
void endOfActivity(const Activity& activity) override;

private:
Quality getSinglePlotQuality(std::shared_ptr<MonitorObject> mo, std::string& message);

std::unique_ptr<ObjectComparatorInterface> mComparator;
std::map<std::string, Quality> mQualityFlags;
std::map<std::string, std::shared_ptr<TPaveText>> mQualityLabels;
quality_control::core::Activity mActivity;
};

} // namespace o2::quality_control_modules::common
Expand Down
5 changes: 0 additions & 5 deletions Modules/Common/include/Common/ReferenceComparatorTask.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@
#define QUALITYCONTROL_REFERENCECOMPARATORTASK_H

#include "Common/ReferenceComparatorTaskConfig.h"
#include "Common/BigScreenCanvas.h"
#include "QualityControl/PostProcessingInterface.h"
#include "QualityControl/DatabaseInterface.h"
#include "QualityControl/Quality.h"
#include <TH1.h>
#include <TPad.h>
#include <TCanvas.h>
#include <TPaveText.h>
#include <TText.h>
#include <string>
#include <map>
Expand Down Expand Up @@ -63,8 +60,6 @@ class ReferenceComparatorTask : public quality_control::postprocessing::PostProc
};

private:
std::shared_ptr<o2::quality_control::core::MonitorObject> getReferencePlot(o2::quality_control::repository::DatabaseInterface& qcdb, std::string fullPath, o2::quality_control::core::Activity activity);

int mReferenceRun{ 0 };
int mNotOlderThan{ 120 };

Expand Down
111 changes: 77 additions & 34 deletions Modules/Common/src/ReferenceComparatorCheck.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
///

#include "Common/ReferenceComparatorCheck.h"
#include "QualityControl/ReferenceUtils.h"
#include "Common/TH1Ratio.h"
#include "Common/TH2Ratio.h"
#include "QualityControl/MonitorObject.h"
#include "QualityControl/Quality.h"
#include "QualityControl/QcInfoLogger.h"
Expand All @@ -25,6 +25,8 @@
#include <DataFormatsQualityControl/FlagType.h>
#include <DataFormatsQualityControl/FlagTypeFactory.h>

#include <Framework/ServiceRegistryRef.h>

// ROOT
#include <TH1.h>
#include <TCanvas.h>
Expand Down Expand Up @@ -54,6 +56,8 @@ void ReferenceComparatorCheck::startOfActivity(const Activity& activity)
if (mComparator) {
mComparator->setThreshold(threshold);
}

mActivity = activity;
}

void ReferenceComparatorCheck::endOfActivity(const Activity& activity)
Expand Down Expand Up @@ -129,6 +133,32 @@ static Quality compare(TCanvas* canvas, ObjectComparatorInterface* comparator, s
return comparator->compare(plots.first, plots.second, message);
}

Quality ReferenceComparatorCheck::getSinglePlotQuality(std::shared_ptr<MonitorObject> mo, std::string& message)
{
// retrieve the reference plot and compare
auto* th1 = dynamic_cast<TH1*>(mo->getObject());
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved
auto referenceRun = std::stoi(mCustomParameters.atOptional("referenceRun").value_or("0"));
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved

// get path of mo and ref (we have to remove the provenance)
string path = mo->getPath();
size_t pos = path.find('/');
if (pos != std::string::npos) {
path = path.substr(pos + 1);
}
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved

auto referencePlot = retrieveReference(path, referenceRun, mActivity);
if (!referencePlot) {
message = "The reference plot is empty";
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved
return Quality::Null;
}
auto* ref = dynamic_cast<TH1*>(referencePlot->getObject());
if (!ref) {
message = "The reference plot is not a TH1";
return Quality::Null;
}
return mComparator.get()->compare(th1, ref, message);
}

//_________________________________________________________________________________________
//
// Loop over all the input MOs and compare each of them with the corresponding MO from the reference run
Expand All @@ -139,17 +169,22 @@ Quality ReferenceComparatorCheck::check(std::map<std::string, std::shared_ptr<Mo
Quality result = Quality::Good;
for (auto& [key, mo] : *moMap) {
auto moName = mo->getName();
Quality quality;
std::string message;

auto* canvas = dynamic_cast<TCanvas*>(mo->getObject());
if (!canvas) {
// run the comparison algorithm
if (mo->getObject()->IsA() == TClass::GetClass<TCanvas>()) {
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved
// We got a canvas. It contains the plot and its reference.
auto* canvas = dynamic_cast<TCanvas*>(mo->getObject());
quality = compare(canvas, mComparator.get(), message);
} else if (mo->getObject()->InheritsFrom(TH1::Class())) {
// We got a plot, we have to find the reference before calling the comparator
quality = getSinglePlotQuality(mo, message);
} else {
ILOG(Warning, Ops) << "mo is not a TCanvas or a TH1" << ENDM;
Barthelemy marked this conversation as resolved.
Show resolved Hide resolved
continue;
}

// run the comparison algorithm
Quality quality;
std::string message;
quality = compare(canvas, mComparator.get(), message);

// update the overall quality
if (quality.isWorseThan(result)) {
result.set(quality);
Expand Down Expand Up @@ -191,6 +226,23 @@ static int getQualityColor(const Quality& q)
return 0;
}

static void updateQualityLabel(TPaveText* label, const Quality& quality)
{
// draw the quality label with the text color corresponding to the quality level
label->SetTextColor(getQualityColor(quality));
label->AddText(quality.getName().c_str());

// add the first flag below the quality label, or an empty line if no flags are set
auto flags = quality.getFlags();
std::string message = flags.empty() ? "" : flags.front().second;
auto pos = message.find(" ");
if (pos != std::string::npos) {
message.erase(0, pos + 1);
}
auto* text = label->AddText(message.c_str());
text->SetTextColor(kGray + 1);
}

// Write the quality level and flags in the existing PaveText inside the canvas
static void setQualityLabel(TCanvas* canvas, const Quality& quality)
{
Expand All @@ -209,43 +261,34 @@ static void setQualityLabel(TCanvas* canvas, const Quality& quality)
continue;
}

// draw the quality label with the text color corresponding to the quality level
label->SetTextColor(getQualityColor(quality));
label->AddText(quality.getName().c_str());

// add the first flag below the quality label, or an empty line if no flags are set
auto flags = quality.getFlags();
std::string message = flags.empty() ? "" : flags.front().second;
auto pos = message.find(" ");
if (pos != std::string::npos) {
message.erase(0, pos + 1);
}
auto* text = label->AddText(message.c_str());
text->SetTextColor(kGray + 1);

updateQualityLabel(label, quality);
break;
}
}

void ReferenceComparatorCheck::beautify(std::shared_ptr<MonitorObject> mo, Quality checkResult)
{
auto* canvas = dynamic_cast<TCanvas*>(mo->getObject());
if (!canvas) {
return;
}

// get the quality associated to the current MO
auto moName = mo->getName();
auto quality = mQualityFlags[moName];

// retrieve the reference plot from the canvas and set the line color according to the quality
auto plots = getPlotsFromCanvas(canvas);
if (plots.second) {
plots.second->SetLineColor(getQualityColor(quality));
}
auto* canvas = dynamic_cast<TCanvas*>(mo->getObject());
if (canvas) {
// retrieve the reference plot from the canvas and set the line color according to the quality
auto plots = getPlotsFromCanvas(canvas);
if (plots.second) {
plots.second->SetLineColor(getQualityColor(quality));
}

// draw the quality label on the plot
setQualityLabel(canvas, quality);
// draw the quality label on the plot
setQualityLabel(canvas, quality);
}
auto* th1 = dynamic_cast<TH1*>(mo->getObject());
if (th1) {
auto* qualityLabel = new TPaveText(0.75, 0.65, 0.98, 0.75, "brNDC");
updateQualityLabel(qualityLabel, quality);
th1->GetListOfFunctions()->Add(qualityLabel);
}
}

} // namespace o2::quality_control_modules::common
Loading
Loading