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.

Sign up for GitHub

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

Jump to bottom

Implement TH2Poly in DQM Services for HGCal DQM #41932

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Implement TH2Poly in DQM Services for HGCal DQM #41932

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a5ffeba
Implement TH2Poly in DQM Services for HGCal DQM
ywkao Jun 11, 2023
5fa5e4e
Apply changes from code quality checks
ywkao Jun 12, 2023
14e856f
Add a patch based on makortel's comments
ywkao Jun 13, 2023
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
14 changes: 14 additions & 0 deletions DQMServices/Core/interface/DQMStore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,20 @@ namespace dqm {
/* forceReplace */ true);
}
template <typename FUNC = NOOP, std::enable_if_t<not std::is_arithmetic<FUNC>::value, int> = 0>
MonitorElement* book2DPoly(TString const& name,
TString const& title,
double lowX,
double highX,
double lowY,
double highY,
FUNC onbooking = NOOP()) {
return bookME(name, MonitorElementData::Kind::TH2F, [=]() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be

Suggested change
return bookME(name, MonitorElementData::Kind::TH2F, [=]() {
return bookME(name, MonitorElementData::Kind::TH2Poly, [=]() {

?

Copy link
Author

Choose a reason for hiding this comment

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

Indeed, this should be corrected. I will update it in a commit accordingly.

auto th2poly = new TH2Poly(name, title, lowX, highX, lowY, highY);
onbooking(th2poly);
return th2poly;
});
}
template <typename FUNC = NOOP, std::enable_if_t<not std::is_arithmetic<FUNC>::value, int> = 0>
MonitorElement* book2S(TString const& name,
TString const& title,
int nchX,
Expand Down
8 changes: 8 additions & 0 deletions DQMServices/Core/interface/MonitorElement.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@
#include "TH2S.h"
#include "TH2I.h"
#include "TH2D.h"
#include "TH2Poly.h"
#include "TH3F.h"
#include "TProfile.h"
#include "TProfile2D.h"
#include "TObjString.h"
#include "TAxis.h"
#include "TGraph.h"

#include <mutex>
#include <memory>
Expand Down Expand Up @@ -398,6 +400,7 @@ namespace dqm::impl {
virtual const std::string &getStringValue() const;

// non-const -- thread safety and semantical issues
virtual void addBin(TGraph *graph);
virtual void setBinContent(int binx, double content);
virtual void setBinContent(int binx, int biny, double content);
virtual void setBinContent(int binx, int biny, int binz, double content);
Expand Down Expand Up @@ -443,6 +446,7 @@ namespace dqm::impl {
virtual TH2S *getTH2S();
virtual TH2I *getTH2I();
virtual TH2D *getTH2D();
virtual TH2Poly *getTH2Poly();
virtual TH3F *getTH3F();
virtual TProfile *getTProfile();
virtual TProfile2D *getTProfile2D();
Expand Down Expand Up @@ -511,6 +515,10 @@ namespace dqm::legacy {
virtual TH2D *getTH2D() const {
return const_cast<dqm::legacy::MonitorElement *>(this)->dqm::reco::MonitorElement::getTH2D();
};
using dqm::reco::MonitorElement::getTH2Poly;
virtual TH2Poly *getTH2Poly() const {
return const_cast<dqm::legacy::MonitorElement *>(this)->dqm::reco::MonitorElement::getTH2Poly();
};
using dqm::reco::MonitorElement::getTH3F;
virtual TH3F *getTH3F() const {
return const_cast<dqm::legacy::MonitorElement *>(this)->dqm::reco::MonitorElement::getTH3F();
Expand Down
16 changes: 16 additions & 0 deletions DQMServices/Core/src/MonitorElement.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ namespace dqm::impl {
static_cast<TH2D *>(accessRootObject(access, __PRETTY_FUNCTION__, 2))->Fill(x, yw, 1);
else if (kind() == Kind::TH2I)
static_cast<TH2I *>(accessRootObject(access, __PRETTY_FUNCTION__, 2))->Fill(x, yw, 1);
else if (kind() == Kind::TH2Poly)
static_cast<TH2Poly *>(accessRootObject(access, __PRETTY_FUNCTION__, 2))->Fill(x, yw, 1);
else if (kind() == Kind::TPROFILE)
static_cast<TProfile *>(accessRootObject(access, __PRETTY_FUNCTION__, 1))->Fill(x, yw, 1);
else
Expand Down Expand Up @@ -329,6 +331,8 @@ namespace dqm::impl {
static_cast<TH2D *>(accessRootObject(access, __PRETTY_FUNCTION__, 2))->Fill(x, y, zw);
else if (kind() == Kind::TH2I)
static_cast<TH2I *>(accessRootObject(access, __PRETTY_FUNCTION__, 2))->Fill(x, y, zw);
else if (kind() == Kind::TH2Poly)
static_cast<TH2Poly *>(accessRootObject(access, __PRETTY_FUNCTION__, 2))->Fill(x, y, zw);
else if (kind() == Kind::TH3F)
static_cast<TH3F *>(accessRootObject(access, __PRETTY_FUNCTION__, 2))->Fill(x, y, zw, 1);
else if (kind() == Kind::TPROFILE)
Expand Down Expand Up @@ -688,6 +692,12 @@ namespace dqm::impl {

/*** setter methods (wrapper around ROOT methods) ****/
//
/// set polygon bin (TH2Poly)
void MonitorElement::addBin(TGraph *graph) {
auto access = this->accessMut();
static_cast<TH2Poly *>(accessRootObject(access, __PRETTY_FUNCTION__, 2))->AddBin(graph);
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the ROOT object is not TH2Poly? The other similar functions throw an exception via the incompatible() function.

Copy link
Author

@ywkao ywkao Jun 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Indeed, incompatible() function is necessary. This block of code will be modified as follows:

-    static_cast<TH2Poly *>(accessRootObject(access, __PRETTY_FUNCTION__, 2))->AddBin(graph);
+    if (kind() == Kind::TH2Poly) {
+      static_cast<TH2Poly *>(accessRootObject(access, __PRETTY_FUNCTION__, 2))->AddBin(graph);
+    } else {
+      incompatible(__PRETTY_FUNCTION__);
+    }

}

/// set content of bin (1-D)
void MonitorElement::setBinContent(int binx, double content) {
auto access = this->accessMut();
Expand Down Expand Up @@ -1032,6 +1042,12 @@ namespace dqm::impl {
return static_cast<TH2D *>(accessRootObject(access, __PRETTY_FUNCTION__, 2));
}

TH2Poly *MonitorElement::getTH2Poly() {
auto access = this->accessMut();
assert(kind() == Kind::TH2Poly);
return static_cast<TH2Poly *>(accessRootObject(access, __PRETTY_FUNCTION__, 2));
}

TH3F *MonitorElement::getTH3F() {
auto access = this->accessMut();
assert(kind() == Kind::TH3F);
Expand Down
1 change: 1 addition & 0 deletions DataFormats/Histograms/interface/MEtoEDMFormat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <TH2F.h>
#include <TH2S.h>
#include <TH2D.h>
#include <TH2Poly.h>
#include <TH3F.h>
#include <TProfile.h>
#include <TProfile2D.h>
Expand Down
3 changes: 2 additions & 1 deletion DataFormats/Histograms/interface/MonitorElementCollection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ struct MonitorElementData {
TH2I = 0x23,
TH3F = 0x30,
TPROFILE = 0x40,
TPROFILE2D = 0x41
TPROFILE2D = 0x41,
TH2Poly = 0x60
Copy link
Contributor

Choose a reason for hiding this comment

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

Why such a large gap between the previous value (0x41)?

Copy link
Author

Choose a reason for hiding this comment

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

I guess I kept thinking of hexagons for a wafer map and then put a number starting with six. We can assign 0x24 for TH2Poly if it is feasible.

+    TH2Poly = 0x24,
     TH3F = 0x30,
     TPROFILE = 0x40,
-    TPROFILE2D = 0x41,
-    TH2Poly = 0x60
+    TPROFILE2D = 0x41

};

// Which window of time the ME is supposed to cover.
Expand Down