Merge pull request #48575 from Dr15Jones/fixHarvestingSumw2

Removed call to TH1::SetDefaultSumw2 found in harvesting step

Author: cmsbuild

DQMOffline/MuonDPG/plugins/TnPEfficiencyClient.cc

@@ -66,11 +66,7 @@ void TnPEfficiencyClient::dqmEndJob(DQMStore::IBooker& ibooker, DQMStore::IGette
   ibooker.setCurrentFolder(topFolder() + outFolder + "/");
   std::string baseFolder = topFolder() + outFolder + "/";
 
-  TH1::SetDefaultSumw2(kTRUE);
-
   for (const auto& s : passNfailHistoNames) {
-    TH1::SetDefaultSumw2(kTRUE);
-
     std::string passHistoName = s.substr(0, s.find(':'));
     std::string failHistoName = s.substr(s.find(':') + 1, s.length());
 
@@ -110,16 +106,20 @@ void TnPEfficiencyClient::dqmEndJob(DQMStore::IBooker& ibooker, DQMStore::IGette
         return;
       }
 
-      TH1F* h1_den = (TH1F*)h1_pass->Clone();
-      TH1F* h1_num = (TH1F*)h1_pass->Clone();
-      h1_den->Sumw2();
-      h1_num->Sumw2();
+      std::unique_ptr<TH1F> h1_den{(TH1F*)h1_pass->Clone()};
+      std::unique_ptr<TH1F> h1_num{(TH1F*)h1_pass->Clone()};
+      //If we call Sumw2 on a histogram which already has it set, we get an exception
+      if (h1_den->GetSumw2N() == 0) {
+        h1_den->Sumw2();
+      }
+      if (h1_num->GetSumw2N() == 0) {
+        h1_num->Sumw2();
+      }
       h1_den->Add(h1_fail);
 
-      h1_num->Divide(h1_den);
-      TH1F* h1_ratio = (TH1F*)h1_num->Clone();
+      h1_num->Divide(h1_den.get());
 
-      effHistos[effHistoName] = ibooker.book1D(effHistoName, h1_ratio);
+      effHistos[effHistoName] = ibooker.book1D(effHistoName, h1_num.get());
       effHistos[effHistoName]->setTitle(effHistoName);
       effHistos[effHistoName]->setAxisTitle("Efficiency", 2);
     }
@@ -146,16 +146,19 @@ void TnPEfficiencyClient::dqmEndJob(DQMStore::IBooker& ibooker, DQMStore::IGette
         return;
       }
 
-      TH2F* h2_den = (TH2F*)h2_pass->Clone();
-      TH2F* h2_num = (TH2F*)h2_pass->Clone();
-      h2_den->Sumw2();
-      h2_num->Sumw2();
+      std::unique_ptr<TH2F> h2_den{(TH2F*)h2_pass->Clone()};
+      std::unique_ptr<TH2F> h2_num{(TH2F*)h2_pass->Clone()};
+      if (h2_den->GetSumw2N() == 0) {
+        h2_den->Sumw2();
+      }
+      if (h2_num->GetSumw2N() == 0) {
+        h2_num->Sumw2();
+      }
       h2_den->Add(h2_fail);
 
-      h2_num->Divide(h2_den);
-      TH2F* h2_ratio = (TH2F*)h2_num->Clone();
+      h2_num->Divide(h2_den.get());
 
-      effHistos[effHistoName] = ibooker.book2D(effHistoName, h2_ratio);
+      effHistos[effHistoName] = ibooker.book2D(effHistoName, h2_num.get());
       effHistos[effHistoName]->setTitle(effHistoName);
       effHistos[effHistoName]->setAxisTitle("Efficiency", 3);
     }

DQMOffline/PFTau/plugins/PFClient.cc

@@ -3,6 +3,7 @@
 #include "DataFormats/Common/interface/Handle.h"
 #include "FWCore/Framework/interface/Event.h"
 #include "FWCore/ParameterSet/interface/ParameterSet.h"
+#include "FWCore/MessageLogger/interface/MessageLogger.h"
 #include "FWCore/Utilities/interface/InputTag.h"
 
 #include "DQMServices/Core/interface/DQMStore.h"
@@ -125,43 +126,47 @@ void PFClient::createResolutionPlots(DQMStore::IBooker &ibooker,
     float ymax = th->GetYaxis()->GetXmax();
     std::string xtit = th->GetXaxis()->GetTitle();
     std::string ytit = th->GetYaxis()->GetTitle();
-    float *xbins = new float[nbinx + 1];
+    std::unique_ptr<float[]> xbins{new float[nbinx + 1]};
     for (size_t ix = 1; ix < nbinx + 1; ++ix) {
       xbins[ix - 1] = th->GetXaxis()->GetBinLowEdge(ix);
       if (ix == nbinx)
         xbins[ix] = th->GetXaxis()->GetBinUpEdge(ix);
     }
     std::string tit_new = ";" + xtit + ";" + ytit;
     ibooker.setCurrentFolder(folder);
-    MonitorElement *me_slice = ibooker.book1D("PFlowSlice", "PFlowSlice", nbiny, ymin, ymax);
+    TH1F th_slice("PFlowSlice", "PFlowSlice", nbiny, ymin, ymax);
 
     tit_new = ";" + xtit + ";Average_" + ytit;
-    me_average = ibooker.book1D("average_" + name, tit_new, nbinx, xbins);
+    me_average = ibooker.book1D("average_" + name, tit_new, nbinx, xbins.get());
     me_average->setEfficiencyFlag();
     tit_new = ";" + xtit + ";RMS_" + ytit;
-    me_rms = ibooker.book1D("rms_" + name, tit_new, nbinx, xbins);
+    me_rms = ibooker.book1D("rms_" + name, tit_new, nbinx, xbins.get());
     me_rms->setEfficiencyFlag();
     tit_new = ";" + xtit + ";Mean_" + ytit;
-    me_mean = ibooker.book1D("mean_" + name, tit_new, nbinx, xbins);
+    me_mean = ibooker.book1D("mean_" + name, tit_new, nbinx, xbins.get());
     me_mean->setEfficiencyFlag();
     tit_new = ";" + xtit + ";Sigma_" + ytit;
-    me_sigma = ibooker.book1D("sigma_" + name, tit_new, nbinx, xbins);
+    me_sigma = ibooker.book1D("sigma_" + name, tit_new, nbinx, xbins.get());
     me_sigma->setEfficiencyFlag();
 
     double average, rms, mean, sigma;
 
     for (size_t ix = 1; ix < nbinx + 1; ++ix) {
-      me_slice->Reset();
+      th_slice.Reset();
+      unsigned int nNonZeroBins = 0;
       for (size_t iy = 1; iy < nbiny + 1; ++iy) {
-        me_slice->setBinContent(iy, th->GetBinContent(ix, iy));
+        auto value = th->GetBinContent(ix, iy);
+        th_slice.SetBinContent(iy, value);
+        if (value != 0) {
+          ++nNonZeroBins;
+        }
       }
-      getHistogramParameters(me_slice, average, rms, mean, sigma);
+      getHistogramParameters(th_slice, nNonZeroBins, average, rms, mean, sigma);
       me_average->setBinContent(ix, average);
       me_rms->setBinContent(ix, rms);
       me_mean->setBinContent(ix, mean);
       me_sigma->setBinContent(ix, sigma);
     }
-    delete[] xbins;
   }
 }
 
@@ -257,7 +262,7 @@ void PFClient::createProfilePlots(DQMStore::IBooker &ibooker,
     // size_t nbiny = me->getNbinsY();
     // TProfile* profileX = th->ProfileX("",0,nbiny+1); add underflow and
     // overflow
-    static const Int_t NUM_STAT = 7;
+    static constexpr Int_t NUM_STAT = 7;
     Double_t stats[NUM_STAT] = {0};
     th->GetStats(stats);
 
@@ -285,24 +290,24 @@ void PFClient::createProfilePlots(DQMStore::IBooker &ibooker,
 // -- Get Histogram Parameters
 //
 void PFClient::getHistogramParameters(
-    MonitorElement *me_slice, double &average, double &rms, double &mean, double &sigma) {
+    TH1F &th_slice, unsigned int nNonZeroBins, double &average, double &rms, double &mean, double &sigma) {
   average = 0.0;
   rms = 0.0;
   mean = 0.0;
   sigma = 0.0;
 
-  if (!me_slice)
-    return;
-  if (me_slice->kind() == MonitorElement::Kind::TH1F) {
-    average = me_slice->getMean();
-    rms = me_slice->getRMS();
-    TH1F *th_slice = me_slice->getTH1F();
-    if (th_slice && th_slice->GetEntries() > 0) {
-      // need our own copy for thread safety
-      TF1 gaus("mygaus", "gaus");
-      th_slice->Fit(&gaus, "Q0 SERIAL");
+  average = th_slice.GetMean();
+  rms = th_slice.GetRMS();
+  //Need a minimum of 3 points to fit a gaussian
+  if (nNonZeroBins > 2) {
+    // need our own copy for thread safety
+    TF1 gaus("mygaus", "gaus", 0, 1, TF1::EAddToList::kNo);
+    auto fit = th_slice.Fit(&gaus, "Q0 SERIAL");
+    if (int(fit) == 0) {
       sigma = gaus.GetParameter(2);
       mean = gaus.GetParameter(1);
+    } else {
+      edm::LogWarning("FitFailed") << "Failed to fit gaussian";
     }
   }
 }

DQMOffline/PFTau/plugins/PFClient.h

@@ -20,7 +20,8 @@ class PFClient : public DQMEDHarvester {
   void doProjection(DQMStore::IBooker &, DQMStore::IGetter &);
   void doProfiles(DQMStore::IBooker &, DQMStore::IGetter &);
   void createResolutionPlots(DQMStore::IBooker &, DQMStore::IGetter &, std::string &folder, std::string &name);
-  void getHistogramParameters(MonitorElement *me_slice, double &avarage, double &rms, double &mean, double &sigma);
+  void getHistogramParameters(
+      TH1F &th_slice, unsigned int nNonZeroBins, double &avarage, double &rms, double &mean, double &sigma);
   void createEfficiencyPlots(DQMStore::IBooker &, DQMStore::IGetter &, std::string &folder, std::string &name);
 
   void createProjectionPlots(DQMStore::IBooker &, DQMStore::IGetter &, std::string &folder, std::string &name);