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

Fix thread-safety issues in TrackingMonitor #29143

Merged
merged 2 commits into from Mar 9, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
130 changes: 52 additions & 78 deletions DQM/TrackingMonitor/src/TrackAnalyzer.cc
Expand Up @@ -2016,25 +2016,23 @@ void TrackAnalyzer::fillHistosForState(const edm::EventSetup& iSetup, const reco
tkmes.TrackEtaPhiInverted->Fill(eta, -1 * phi);
tkmes.TrackEtaPhiInvertedoutofphase->Fill(eta, 3.141592654 + -1 * phi);
tkmes.TrackEtaPhiInvertedoutofphase->Fill(eta, -1 * phi - 3.141592654);
tkmes.TkEtaPhi_Ratio_byFoldingmap->getTH2F()->Divide(
tkmes.TrackEtaPhi->getTH2F(), tkmes.TrackEtaPhiInverted->getTH2F(), 1., 1., "");
tkmes.TkEtaPhi_Ratio_byFoldingmap_op->getTH2F()->Divide(
tkmes.TrackEtaPhi->getTH2F(), tkmes.TrackEtaPhiInvertedoutofphase->getTH2F(), 1., 1., "");
tkmes.TkEtaPhi_Ratio_byFoldingmap->divide(tkmes.TrackEtaPhi, tkmes.TrackEtaPhiInverted, 1., 1., "");
tkmes.TkEtaPhi_Ratio_byFoldingmap_op->divide(tkmes.TrackEtaPhi, tkmes.TrackEtaPhiInvertedoutofphase, 1., 1., "");

int nx = tkmes.TrackEtaPhi->getTH2F()->GetNbinsX();
int ny = tkmes.TrackEtaPhi->getTH2F()->GetNbinsY();
int nx = tkmes.TrackEtaPhi->getNbinsX();
int ny = tkmes.TrackEtaPhi->getNbinsY();

//NOTE: for full reproducibility when using threads, this loop needs to be
// a critical section
Copy link
Contributor

Choose a reason for hiding this comment

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

How important is "full reproducibility when using threads"? (just thinking how soon a solution for that needs to found)

(e.g. are the divisions and get/setBinContent something that needs to be done on every event, or should they move to harvesting?)

for (int ii = 1; ii <= nx; ii++) {
for (int jj = 1; jj <= ny; jj++) {
double Sum1 = tkmes.TrackEtaPhi->getTH2F()->GetBinContent(ii, jj) +
tkmes.TrackEtaPhiInverted->getTH2F()->GetBinContent(ii, jj);
double Sum2 = tkmes.TrackEtaPhi->getTH2F()->GetBinContent(ii, jj) +
tkmes.TrackEtaPhiInvertedoutofphase->getTH2F()->GetBinContent(ii, jj);
double Sum1 = tkmes.TrackEtaPhi->getBinContent(ii, jj) + tkmes.TrackEtaPhiInverted->getBinContent(ii, jj);
double Sum2 =
tkmes.TrackEtaPhi->getBinContent(ii, jj) + tkmes.TrackEtaPhiInvertedoutofphase->getBinContent(ii, jj);

double Sub1 = tkmes.TrackEtaPhi->getTH2F()->GetBinContent(ii, jj) -
tkmes.TrackEtaPhiInverted->getTH2F()->GetBinContent(ii, jj);
double Sub2 = tkmes.TrackEtaPhi->getTH2F()->GetBinContent(ii, jj) -
tkmes.TrackEtaPhiInvertedoutofphase->getTH2F()->GetBinContent(ii, jj);
double Sub1 = tkmes.TrackEtaPhi->getBinContent(ii, jj) - tkmes.TrackEtaPhiInverted->getBinContent(ii, jj);
double Sub2 =
tkmes.TrackEtaPhi->getBinContent(ii, jj) - tkmes.TrackEtaPhiInvertedoutofphase->getBinContent(ii, jj);

if (Sum1 == 0 || Sum2 == 0) {
tkmes.TkEtaPhi_RelativeDifference_byFoldingmap->setBinContent(ii, jj, 1);
Expand Down Expand Up @@ -2076,14 +2074,14 @@ void TrackAnalyzer::fillHistosForState(const edm::EventSetup& iSetup, const reco
}

float A[8];
A[0] = tkmes.TrackPt_NegEta_Phi_btw_neg16_neg32->getTH1()->Integral();
A[1] = tkmes.TrackPt_NegEta_Phi_btw_0_neg16->getTH1()->Integral();
A[2] = tkmes.TrackPt_NegEta_Phi_btw_16_0->getTH1()->Integral();
A[3] = tkmes.TrackPt_NegEta_Phi_btw_32_16->getTH1()->Integral();
A[4] = tkmes.TrackPt_PosEta_Phi_btw_neg16_neg32->getTH1()->Integral();
A[5] = tkmes.TrackPt_PosEta_Phi_btw_0_neg16->getTH1()->Integral();
A[6] = tkmes.TrackPt_PosEta_Phi_btw_16_0->getTH1()->Integral();
A[7] = tkmes.TrackPt_PosEta_Phi_btw_32_16->getTH1()->Integral();
A[0] = tkmes.TrackPt_NegEta_Phi_btw_neg16_neg32->integral();
A[1] = tkmes.TrackPt_NegEta_Phi_btw_0_neg16->integral();
A[2] = tkmes.TrackPt_NegEta_Phi_btw_16_0->integral();
A[3] = tkmes.TrackPt_NegEta_Phi_btw_32_16->integral();
A[4] = tkmes.TrackPt_PosEta_Phi_btw_neg16_neg32->integral();
A[5] = tkmes.TrackPt_PosEta_Phi_btw_0_neg16->integral();
A[6] = tkmes.TrackPt_PosEta_Phi_btw_16_0->integral();
A[7] = tkmes.TrackPt_PosEta_Phi_btw_32_16->integral();

//WZ (the worst zone)
int WZ = 0;
Expand All @@ -2097,76 +2095,52 @@ void TrackAnalyzer::fillHistosForState(const edm::EventSetup& iSetup, const reco

switch (WZ) {
case 1:
tkmes.Ratio_byFolding->getTH1()->Divide(tkmes.TrackPt_NegEta_Phi_btw_neg16_neg32->getTH1(),
tkmes.TrackPt_NegEta_Phi_btw_32_16->getTH1(),
1.,
1.,
"B");
tkmes.Ratio_byFolding2->getTH1()->Divide(tkmes.TrackPt_NegEta_Phi_btw_neg16_neg32->getTH1(),
tkmes.TrackPt_NegEta_Phi_btw_0_neg16->getTH1(),
1.,
1.,
"B");
tkmes.Ratio_byFolding->divide(
tkmes.TrackPt_NegEta_Phi_btw_neg16_neg32, tkmes.TrackPt_NegEta_Phi_btw_32_16, 1., 1., "B");
tkmes.Ratio_byFolding2->divide(
tkmes.TrackPt_NegEta_Phi_btw_neg16_neg32, tkmes.TrackPt_NegEta_Phi_btw_0_neg16, 1., 1., "B");
break;
case 2:
tkmes.Ratio_byFolding->getTH1()->Divide(
tkmes.TrackPt_NegEta_Phi_btw_0_neg16->getTH1(), tkmes.TrackPt_NegEta_Phi_btw_16_0->getTH1(), 1., 1., "B");
tkmes.Ratio_byFolding2->getTH1()->Divide(tkmes.TrackPt_NegEta_Phi_btw_0_neg16->getTH1(),
tkmes.TrackPt_NegEta_Phi_btw_neg16_neg32->getTH1(),
1.,
1.,
"B");
tkmes.Ratio_byFolding->divide(
tkmes.TrackPt_NegEta_Phi_btw_0_neg16, tkmes.TrackPt_NegEta_Phi_btw_16_0, 1., 1., "B");
tkmes.Ratio_byFolding2->divide(
tkmes.TrackPt_NegEta_Phi_btw_0_neg16, tkmes.TrackPt_NegEta_Phi_btw_neg16_neg32, 1., 1., "B");
break;
case 3:
tkmes.Ratio_byFolding->getTH1()->Divide(
tkmes.TrackPt_NegEta_Phi_btw_16_0->getTH1(), tkmes.TrackPt_NegEta_Phi_btw_0_neg16->getTH1(), 1., 1., "B");
tkmes.Ratio_byFolding2->getTH1()->Divide(
tkmes.TrackPt_NegEta_Phi_btw_16_0->getTH1(), tkmes.TrackPt_NegEta_Phi_btw_32_16->getTH1(), 1., 1., "B");
tkmes.Ratio_byFolding->divide(
tkmes.TrackPt_NegEta_Phi_btw_16_0, tkmes.TrackPt_NegEta_Phi_btw_0_neg16, 1., 1., "B");
tkmes.Ratio_byFolding2->divide(
tkmes.TrackPt_NegEta_Phi_btw_16_0, tkmes.TrackPt_NegEta_Phi_btw_32_16, 1., 1., "B");
break;
case 4:
tkmes.Ratio_byFolding->getTH1()->Divide(tkmes.TrackPt_NegEta_Phi_btw_32_16->getTH1(),
tkmes.TrackPt_NegEta_Phi_btw_neg16_neg32->getTH1(),
1.,
1.,
"B");
tkmes.Ratio_byFolding2->getTH1()->Divide(
tkmes.TrackPt_NegEta_Phi_btw_32_16->getTH1(), tkmes.TrackPt_NegEta_Phi_btw_16_0->getTH1(), 1., 1., "B");
tkmes.Ratio_byFolding->divide(
tkmes.TrackPt_NegEta_Phi_btw_32_16, tkmes.TrackPt_NegEta_Phi_btw_neg16_neg32, 1., 1., "B");
tkmes.Ratio_byFolding2->divide(
tkmes.TrackPt_NegEta_Phi_btw_32_16, tkmes.TrackPt_NegEta_Phi_btw_16_0, 1., 1., "B");
break;
case 5:
tkmes.Ratio_byFolding->getTH1()->Divide(tkmes.TrackPt_PosEta_Phi_btw_neg16_neg32->getTH1(),
tkmes.TrackPt_PosEta_Phi_btw_32_16->getTH1(),
1.,
1.,
"B");
tkmes.Ratio_byFolding2->getTH1()->Divide(tkmes.TrackPt_PosEta_Phi_btw_neg16_neg32->getTH1(),
tkmes.TrackPt_PosEta_Phi_btw_0_neg16->getTH1(),
1.,
1.,
"B");
tkmes.Ratio_byFolding->divide(
tkmes.TrackPt_PosEta_Phi_btw_neg16_neg32, tkmes.TrackPt_PosEta_Phi_btw_32_16, 1., 1., "B");
tkmes.Ratio_byFolding2->divide(
tkmes.TrackPt_PosEta_Phi_btw_neg16_neg32, tkmes.TrackPt_PosEta_Phi_btw_0_neg16, 1., 1., "B");
break;
case 6:
tkmes.Ratio_byFolding->getTH1()->Divide(
tkmes.TrackPt_PosEta_Phi_btw_0_neg16->getTH1(), tkmes.TrackPt_PosEta_Phi_btw_16_0->getTH1(), 1., 1., "B");
tkmes.Ratio_byFolding2->getTH1()->Divide(tkmes.TrackPt_PosEta_Phi_btw_0_neg16->getTH1(),
tkmes.TrackPt_PosEta_Phi_btw_neg16_neg32->getTH1(),
1.,
1.,
"B");
tkmes.Ratio_byFolding->divide(
tkmes.TrackPt_PosEta_Phi_btw_0_neg16, tkmes.TrackPt_PosEta_Phi_btw_16_0, 1., 1., "B");
tkmes.Ratio_byFolding2->divide(
tkmes.TrackPt_PosEta_Phi_btw_0_neg16, tkmes.TrackPt_PosEta_Phi_btw_neg16_neg32, 1., 1., "B");
break;
case 7:
tkmes.Ratio_byFolding->getTH1()->Divide(
tkmes.TrackPt_PosEta_Phi_btw_16_0->getTH1(), tkmes.TrackPt_PosEta_Phi_btw_0_neg16->getTH1(), 1., 1., "B");
tkmes.Ratio_byFolding2->getTH1()->Divide(
tkmes.TrackPt_PosEta_Phi_btw_16_0->getTH1(), tkmes.TrackPt_PosEta_Phi_btw_32_16->getTH1(), 1., 1., "B");
tkmes.Ratio_byFolding->divide(
tkmes.TrackPt_PosEta_Phi_btw_16_0, tkmes.TrackPt_PosEta_Phi_btw_0_neg16, 1., 1., "B");
tkmes.Ratio_byFolding2->divide(
tkmes.TrackPt_PosEta_Phi_btw_16_0, tkmes.TrackPt_PosEta_Phi_btw_32_16, 1., 1., "B");
break;
case 8:
tkmes.Ratio_byFolding->getTH1()->Divide(tkmes.TrackPt_PosEta_Phi_btw_32_16->getTH1(),
tkmes.TrackPt_PosEta_Phi_btw_neg16_neg32->getTH1(),
1.,
1.,
"B");
tkmes.Ratio_byFolding2->getTH1()->Divide(
tkmes.TrackPt_PosEta_Phi_btw_32_16->getTH1(), tkmes.TrackPt_PosEta_Phi_btw_16_0->getTH1(), 1., 1., "B");
tkmes.Ratio_byFolding->divide(
tkmes.TrackPt_PosEta_Phi_btw_32_16, tkmes.TrackPt_PosEta_Phi_btw_neg16_neg32, 1., 1., "B");
tkmes.Ratio_byFolding2->divide(
tkmes.TrackPt_PosEta_Phi_btw_32_16, tkmes.TrackPt_PosEta_Phi_btw_16_0, 1., 1., "B");
break;
}
tkmes.Ratio_byFolding->setAxisTitle("Efficiency(Ratio)_" + std::to_string(WZ), 2);
Expand Down
2 changes: 2 additions & 0 deletions DQMServices/Core/interface/MonitorElement.h
Expand Up @@ -387,6 +387,7 @@ namespace dqm::impl {
virtual double getBinError(int binx, int biny, int binz) const;
virtual double getEntries() const;
virtual double getBinEntries(int bin) const;
virtual double integral() const;

virtual int64_t getIntValue() const;
virtual double getFloatValue() const;
Expand All @@ -401,6 +402,7 @@ namespace dqm::impl {
virtual void setBinError(int binx, int biny, int binz, double error);
virtual void setBinEntries(int bin, double nentries);
virtual void setEntries(double nentries);
virtual void divide(const MonitorElement *, const MonitorElement *, double, double, const char *);
virtual void setBinLabel(int bin, const std::string &label, int axis = 1);
virtual void setAxisRange(double xmin, double xmax, int axis = 1);
virtual void setAxisTitle(const std::string &title, int axis = 1);
Expand Down
31 changes: 31 additions & 0 deletions DQMServices/Core/src/MonitorElement.cc
Expand Up @@ -652,6 +652,12 @@ namespace dqm::impl {
}
}

/// get integral of bins
double MonitorElement::integral() const {
auto access = this->access();
return accessRootObject(access, __PRETTY_FUNCTION__, 1)->Integral();
}

/// get x-, y- or z-axis title (axis=1, 2, 3 respectively)
std::string MonitorElement::getAxisTitle(int axis /* = 1 */) const {
auto access = this->access();
Expand Down Expand Up @@ -719,6 +725,31 @@ namespace dqm::impl {
accessRootObject(access, __PRETTY_FUNCTION__, 1)->SetEntries(nentries);
}

/// Replace entries with results of dividing num by denom
void MonitorElement::divide(
const MonitorElement *num, const MonitorElement *denom, double c1, double c2, const char *options) {
if (num->kind() < Kind::TH1F)
num->incompatible(__PRETTY_FUNCTION__);
if (denom->kind() < Kind::TH1F)
denom->incompatible(__PRETTY_FUNCTION__);

TH1 const *numH = static_cast<TH1 const *>(num->getRootObject());
TH1 const *denomH = static_cast<TH1 const *>(denom->getRootObject());
TH1 *thisH = getTH1();

//Need to take locks in a consistent order to avoid deadlocks. Use pointer value order.
//This is known as the monitor pattern.
std::array<const MonitorElement *, 3> order{{this, num, denom}};
std::sort(order.begin(), order.end());

auto a0 = order[0]->access();
auto a1 = order[1]->access();
auto a2 = order[2]->access();

//Have ROOT do check that the types are compatible
thisH->Divide(numH, denomH, c1, c2, options);
}

/// 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 */) {
bool fail = false;
Expand Down