Skip to content

Commit

Permalink
Merge pull request #28916 from schneiml/dqm-fix-deadlock-in-setbinlabel
Browse files Browse the repository at this point in the history 
DQM: Fix potential deadlock in setBinLabel.
  • Loading branch information
@cmsbuild
cmsbuild authored Feb 12, 2020
2 parents 31789b8 + 6d3777a commit 5ebc557
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 88 deletions.
34 changes: 21 additions & 13 deletions DQMServices/Core/src/MonitorElement.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,18 +721,24 @@ namespace dqm::impl {

/// set bin label for x, y or z axis (axis=1, 2, 3 respectively)
void MonitorElement::setBinLabel(int bin, const std::string &label, int axis /* = 1 */) {
auto access = this->accessMut();
update();
if (getAxis(access, __PRETTY_FUNCTION__, axis)->GetNbins() >= bin) {
getAxis(access, __PRETTY_FUNCTION__, axis)->SetBinLabel(bin, label.c_str());
} else {
#if WITHOUT_CMS_FRAMEWORK
std::cout
#else
edm::LogWarning("MonitorElement")
#endif
<< "*** MonitorElement: WARNING:"
<< "setBinLabel: attempting to set label of non-existent bin number for ME: " << getFullname() << " \n";
bool fail = false;
{
auto access = this->accessMut();
update();
if (getAxis(access, __PRETTY_FUNCTION__, axis)->GetNbins() >= bin) {
getAxis(access, __PRETTY_FUNCTION__, axis)->SetBinLabel(bin, label.c_str());
} else {
fail = true;
}
}
// do this with the ME lock released to prevent a deadlock
if (fail) {
// this also takes the lock, make sure to release it before going to edm
// (which might take more locks)
auto name = getFullname();
edm::LogWarning("MonitorElement") << "*** MonitorElement: WARNING:"
<< "setBinLabel: attempting to set label of non-existent bin number for ME: "
<< name << " \n";
}
}

Expand Down Expand Up @@ -822,7 +828,9 @@ namespace dqm::impl {
void MonitorElement::enableSumw2() {
auto access = this->accessMut();
update();
access.value.object_->Sumw2();
if (access.value.object_->GetSumw2() == nullptr) {
access.value.object_->Sumw2();
}
}

void MonitorElement::disableAlphanumeric() {
Expand Down
12 changes: 4 additions & 8 deletions Validation/EventGenerator/src/DQMHelper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,15 @@ DQMHelper::~DQMHelper() {}

DQMHelper::MonitorElement* DQMHelper::book1dHisto(
std::string name, std::string title, int n, double xmin, double xmax, std::string xaxis, std::string yaxis) {
MonitorElement* dqm = ibooker->book1D(name, title, n, xmin, xmax);
dqm->getTH1()->Sumw2();
MonitorElement* dqm = ibooker->book1D(name, title, n, xmin, xmax, [](TH1* th1) { th1->Sumw2(); });
dqm->setAxisTitle(xaxis, 1);
dqm->setAxisTitle(yaxis, 2);
return dqm;
}

DQMHelper::MonitorElement* DQMHelper::book1dHisto(
const std::string& name, const std::string& title, int n, double xmin, double xmax) {
MonitorElement* dqm = ibooker->book1D(name, title, n, xmin, xmax);
dqm->getTH1()->Sumw2();
MonitorElement* dqm = ibooker->book1D(name, title, n, xmin, xmax, [](TH1* th1) { th1->Sumw2(); });
return dqm;
}

Expand All @@ -30,8 +28,7 @@ DQMHelper::MonitorElement* DQMHelper::book2dHisto(std::string name,
double ymax,
std::string xaxis,
std::string yaxis) {
MonitorElement* dqm = ibooker->book2D(name, title, nx, xmin, xmax, ny, ymin, ymax);
dqm->getTH1()->Sumw2();
MonitorElement* dqm = ibooker->book2D(name, title, nx, xmin, xmax, ny, ymin, ymax, [](TH1* th1) { th1->Sumw2(); });
dqm->setAxisTitle(xaxis, 1);
dqm->setAxisTitle(yaxis, 2);
return dqm;
Expand All @@ -45,7 +42,6 @@ DQMHelper::MonitorElement* DQMHelper::book2dHisto(const std::string& name,
int ny,
double ymin,
double ymax) {
MonitorElement* dqm = ibooker->book2D(name, title, nx, xmin, xmax, ny, ymin, ymax);
dqm->getTH1()->Sumw2();
MonitorElement* dqm = ibooker->book2D(name, title, nx, xmin, xmax, ny, ymin, ymax, [](TH1* th1) { th1->Sumw2(); });
return dqm;
}
6 changes: 1 addition & 5 deletions Validation/MuonIsolation/interface/MuIsoValidation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,7 @@ class MuIsoValidation : public DQMEDAnalyzer {
void bookHistograms(DQMStore::IBooker&, edm::Run const&, edm::EventSetup const&) override;

void MakeLogBinsForProfile(Double_t* bin_edges, const double min, const double max);
void FillHistos(); //Fills histograms with data
void NormalizeHistos(); //Normalize to number of muons
TH1* GetTH1FromMonitorElement(MonitorElement* me);
TH2* GetTH2FromMonitorElement(MonitorElement* me);
TProfile* GetTProfileFromMonitorElement(MonitorElement* me);
void FillHistos(); //Fills histograms with data

//----------Static Variables---------------

Expand Down
80 changes: 18 additions & 62 deletions Validation/MuonIsolation/src/MuIsoValidation.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,21 +390,24 @@ void MuIsoValidation::bookHistograms(DQMStore::IBooker& ibooker, edm::Run const&

//---Initialize 1D Histograms---
for (int var = 0; var < NUM_VARS; var++) {
h_1D[var] = ibooker.book1D(
names[var], title_sam + main_titles[var] + title_cone, (int)param[var][0], param[var][1], param[var][2]);
h_1D[var] = ibooker.book1D(names[var],
title_sam + main_titles[var] + title_cone,
(int)param[var][0],
param[var][1],
param[var][2],
[](TH1* th1) { th1->Sumw2(); });
h_1D[var]->setAxisTitle(axis_titles[var], XAXIS);
h_1D[var]->setAxisTitle("Fraction of Muons", YAXIS);
GetTH1FromMonitorElement(h_1D[var])->Sumw2();

if (cdCompNeeded[var]) {
cd_plots[var] = ibooker.book1D(names[var] + "_cd",
title_sam + title_cd + main_titles[var] + title_cone,
(int)param[var][0],
param[var][1],
param[var][2]);
param[var][2],
[](TH1* th1) { th1->Sumw2(); });
cd_plots[var]->setAxisTitle(axis_titles[var], XAXIS);
cd_plots[var]->setAxisTitle("Fraction of Muons", YAXIS);
GetTH1FromMonitorElement(cd_plots[var])->Sumw2();
}
} //Finish 1D

Expand All @@ -414,18 +417,6 @@ void MuIsoValidation::bookHistograms(DQMStore::IBooker& ibooker, edm::Run const&
if (var1 == var2)
continue;

/* h_2D[var1][var2] = ibooker.book2D(
names[var1] + "_" + names[var2] + "_s",
//title is in "y-var vs. x-var" format
title_sam + main_titles[var2] + " <vs> " + main_titles[var1] + title_cone,
(int)param[var1][0],
param[var1][1],
param[var1][2],
(int)param[var2][0],
param[var2][1],
param[var2][2]
);
*/
//Monitor elements is weird and takes y axis parameters as well
//as x axis parameters for a 1D profile plot
p_2D[var1][var2] = ibooker.bookProfile(names[var1] + "_" + names[var2],
Expand All @@ -436,24 +427,19 @@ void MuIsoValidation::bookHistograms(DQMStore::IBooker& ibooker, edm::Run const&
(int)param[var2][0], //documentation says this is disregarded
param[var2][1], //does this do anything?
param[var2][2], //does this do anything?
" " //profile errors = spread/sqrt(num_datums)
);

if (LOG_BINNING_ENABLED && isContinuous[var1]) {
Double_t* bin_edges = new Double_t[NUM_LOG_BINS + 1];
// nbins+1 because there is one more edge than there are bins
MakeLogBinsForProfile(bin_edges, param[var1][1], param[var1][2]);
GetTProfileFromMonitorElement(p_2D[var1][var2])->SetBins(NUM_LOG_BINS, bin_edges);
delete[] bin_edges;
}
/* h_2D[var1][var2]->setAxisTitle(axis_titles[var1],XAXIS);
h_2D[var1][var2]->setAxisTitle(axis_titles[var2],YAXIS);
GetTH2FromMonitorElement(h_2D[var1][var2])->Sumw2();
*/
" ", //profile errors = spread/sqrt(num_datums)
[&](TProfile* tprof) {
if (LOG_BINNING_ENABLED && isContinuous[var1]) {
Double_t* bin_edges = new Double_t[NUM_LOG_BINS + 1];
// nbins+1 because there is one more edge than there are bins
MakeLogBinsForProfile(bin_edges, param[var1][1], param[var1][2]);
tprof->SetBins(NUM_LOG_BINS, bin_edges);
delete[] bin_edges;
}
});

p_2D[var1][var2]->setAxisTitle(axis_titles[var1], XAXIS);
p_2D[var1][var2]->setAxisTitle(axis_titles[var2], YAXIS);
// GetTProfileFromMonitorElement(p_2D[var1][var2])->Sumw2();
}
} //Finish 2D

Expand Down Expand Up @@ -481,30 +467,6 @@ void MuIsoValidation::MakeLogBinsForProfile(Double_t* bin_edges, const double mi
bin_edges[nbins] = max;
}

void MuIsoValidation::NormalizeHistos() {
for (int var = 0; var < NUM_VARS; var++) {
//turn cd_plots into CDF's
//underflow -> bin #0. overflow -> bin #(nbins+1)
//0th bin doesn't need changed

//----normalize------
double entries = GetTH1FromMonitorElement(h_1D[var])->GetEntries();
if (entries == 0)
continue;
GetTH1FromMonitorElement(h_1D[var])->Scale(1. / entries);

if (cdCompNeeded[var]) {
int n_max = int(param[var][0]) + 1;
for (int n = 1; n <= n_max; ++n) {
cd_plots[var]->setBinContent(
n, cd_plots[var]->getBinContent(n) + cd_plots[var]->getBinContent(n - 1)); //Integrate.
}
//----normalize------
GetTH1FromMonitorElement(cd_plots[var])->Scale(1. / entries);
}
}
}

void MuIsoValidation::FillHistos() {
//----------Fill 1D histograms---------------
for (int var = 0; var < NUM_VARS; var++) {
Expand All @@ -525,11 +487,5 @@ void MuIsoValidation::FillHistos() {
} //Finish 2D
}

TH1* MuIsoValidation::GetTH1FromMonitorElement(MonitorElement* me) { return me->getTH1(); }

TH2* MuIsoValidation::GetTH2FromMonitorElement(MonitorElement* me) { return me->getTH2F(); }

TProfile* MuIsoValidation::GetTProfileFromMonitorElement(MonitorElement* me) { return me->getTProfile(); }

//define this as a plug-in
DEFINE_FWK_MODULE(MuIsoValidation);

0 comments on commit 5ebc557

Please sign in to comment.