Skip to content

Commit e568dd9

Browse files
guitargeekguitargeek
committed
[RF] Replace more usage of non-threadsafe Form() 
1 parent a2acd4b commit e568dd9

File tree

5 files changed

+105
-134
lines changed

5 files changed

+105
-134
lines changed

‎roofit/roofitcore/src/RooAbsData.cxx

Lines changed: 58 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1942,6 +1942,44 @@ RooPlot *RooAbsData::plotOn(RooPlot *frame, PlotOpt o) const
19421942
return frame;
19431943
}
19441944

1945+
namespace {
1946+
1947+
RooHist *createAndFillRooHist(RooAbsData const &absData, RooPlot const &frame, RooAbsRealLValue const &var,
1948+
std::string cuts1, std::string cuts2, RooAbsData::PlotOpt opt, bool efficiency,
1949+
double scaleFactor)
1950+
{
1951+
// create and fill temporary histograms of this variable for each state
1952+
std::string hist1Name = std::string{absData.GetName()} + "_plot_1";
1953+
std::string hist2Name = std::string{absData.GetName()} + "_plot_2";
1954+
std::unique_ptr<TH1> hist1;
1955+
std::unique_ptr<TH1> hist2;
1956+
1957+
if (opt.bins) {
1958+
hist1.reset(var.createHistogram(hist1Name.c_str(), "Events", *opt.bins));
1959+
hist2.reset(var.createHistogram(hist2Name.c_str(), "Events", *opt.bins));
1960+
} else {
1961+
auto &axis = *frame.GetXaxis();
1962+
hist1.reset(var.createHistogram(hist1Name.c_str(), "Events", axis.GetXmin(), axis.GetXmax(), frame.GetNbinsX()));
1963+
hist2.reset(var.createHistogram(hist2Name.c_str(), "Events", axis.GetXmin(), axis.GetXmax(), frame.GetNbinsX()));
1964+
}
1965+
1966+
if (opt.cuts && strlen(opt.cuts)) {
1967+
std::string cuts = opt.cuts;
1968+
cuts1 += "&&(" + cuts + ")";
1969+
cuts2 += "&&(" + cuts + ")";
1970+
}
1971+
1972+
if (!absData.fillHistogram(hist1.get(), RooArgList(var), cuts1.c_str(), opt.cutRange) ||
1973+
!absData.fillHistogram(hist2.get(), RooArgList(var), cuts2.c_str(), opt.cutRange)) {
1974+
return nullptr;
1975+
}
1976+
1977+
// convert this histogram to a RooHist object on the heap
1978+
return new RooHist(*hist1, *hist2, 0, 1, opt.etype, opt.xErrorSize, efficiency, scaleFactor);
1979+
}
1980+
1981+
} // namespace
1982+
19451983
////////////////////////////////////////////////////////////////////////////////
19461984
/// Create and fill a histogram with the asymmetry N[+] - N[-] / ( N[+] + N[-] ),
19471985
/// where N(+/-) is the number of data points with asymCat=+1 and asymCat=-1
@@ -1969,47 +2007,13 @@ RooPlot* RooAbsData::plotAsymOn(RooPlot* frame, const RooAbsCategoryLValue& asym
19692007
return nullptr;
19702008
}
19712009

1972-
// create and fill temporary histograms of this variable for each state
1973-
std::string hist1Name(GetName());
1974-
std::string hist2Name(GetName());
1975-
hist1Name += "_plot1";
1976-
std::unique_ptr<TH1> hist1;
1977-
std::unique_ptr<TH1> hist2;
1978-
hist2Name += "_plot2";
1979-
1980-
if (o.bins) {
1981-
hist1.reset( var->createHistogram(hist1Name.c_str(), "Events", *o.bins) );
1982-
hist2.reset( var->createHistogram(hist2Name.c_str(), "Events", *o.bins) );
1983-
} else {
1984-
hist1.reset( var->createHistogram(hist1Name.c_str(), "Events",
1985-
frame->GetXaxis()->GetXmin(), frame->GetXaxis()->GetXmax(),
1986-
frame->GetNbinsX()) );
1987-
hist2.reset( var->createHistogram(hist2Name.c_str(), "Events",
1988-
frame->GetXaxis()->GetXmin(), frame->GetXaxis()->GetXmax(),
1989-
frame->GetNbinsX()) );
1990-
}
1991-
1992-
assert(hist1 && hist2);
1993-
1994-
std::string cuts1;
1995-
std::string cuts2;
1996-
if (o.cuts && strlen(o.cuts)) {
1997-
cuts1 = Form("(%s)&&(%s>0)",o.cuts,asymCat.GetName());
1998-
cuts2 = Form("(%s)&&(%s<0)",o.cuts,asymCat.GetName());
1999-
} else {
2000-
cuts1 = Form("(%s>0)",asymCat.GetName());
2001-
cuts2 = Form("(%s<0)",asymCat.GetName());
2002-
}
2003-
2004-
if(! fillHistogram(hist1.get(), RooArgList(*var),cuts1.c_str(),o.cutRange) ||
2005-
! fillHistogram(hist2.get(), RooArgList(*var),cuts2.c_str(),o.cutRange)) {
2006-
coutE(Plotting) << ClassName() << "::" << GetName()
2007-
<< ":plotAsymOn: createHistogram() failed" << std::endl;
2008-
return nullptr;
2010+
std::string catName = asymCat.GetName();
2011+
RooHist *graph =
2012+
createAndFillRooHist(*this, *frame, *var, "(" + catName + ">0)", "(" + catName + "<0)", o, false, o.scaleFactor);
2013+
if (graph == nullptr) {
2014+
coutE(Plotting) << ClassName() << "::" << GetName() << ":plotAsymOn: createHistogram() failed" << std::endl;
2015+
return nullptr;
20092016
}
2010-
2011-
// convert this histogram to a RooHist object on the heap
2012-
RooHist *graph= new RooHist(*hist1,*hist2,0,1,o.etype,o.xErrorSize,false,o.scaleFactor);
20132017
graph->setYAxisLabel((std::string{"Asymmetry in "} + asymCat.GetName()).c_str());
20142018

20152019
// initialize the frame's normalization setup, if necessary
@@ -2063,48 +2067,16 @@ RooPlot* RooAbsData::plotEffOn(RooPlot* frame, const RooAbsCategoryLValue& effCa
20632067
return nullptr;
20642068
}
20652069

2066-
// create and fill temporary histograms of this variable for each state
2067-
std::string hist1Name(GetName());
2068-
std::string hist2Name(GetName());
2069-
hist1Name += "_plot1";
2070-
std::unique_ptr<TH1> hist1;
2071-
std::unique_ptr<TH1> hist2;
2072-
hist2Name += "_plot2";
2070+
std::string catName = effCat.GetName();
2071+
RooHist *graph =
2072+
createAndFillRooHist(*this, *frame, *var, "(" + catName + "==1)", "(" + catName + "==0)", o, true, 1.0);
20732073

2074-
if (o.bins) {
2075-
hist1.reset( var->createHistogram(hist1Name.c_str(), "Events", *o.bins) );
2076-
hist2.reset( var->createHistogram(hist2Name.c_str(), "Events", *o.bins) );
2077-
} else {
2078-
hist1.reset( var->createHistogram(hist1Name.c_str(), "Events",
2079-
frame->GetXaxis()->GetXmin(), frame->GetXaxis()->GetXmax(),
2080-
frame->GetNbinsX()) );
2081-
hist2.reset( var->createHistogram(hist2Name.c_str(), "Events",
2082-
frame->GetXaxis()->GetXmin(), frame->GetXaxis()->GetXmax(),
2083-
frame->GetNbinsX()) );
2074+
if (graph == nullptr) {
2075+
coutE(Plotting) << ClassName() << "::" << GetName() << ":plotEffOn: createHistogram() failed" << std::endl;
2076+
return nullptr;
20842077
}
20852078

2086-
assert(hist1 && hist2);
2087-
2088-
std::string cuts1;
2089-
std::string cuts2;
2090-
if (o.cuts && strlen(o.cuts)) {
2091-
cuts1 = Form("(%s)&&(%s==1)",o.cuts,effCat.GetName());
2092-
cuts2 = Form("(%s)&&(%s==0)",o.cuts,effCat.GetName());
2093-
} else {
2094-
cuts1 = Form("(%s==1)",effCat.GetName());
2095-
cuts2 = Form("(%s==0)",effCat.GetName());
2096-
}
2097-
2098-
if(! fillHistogram(hist1.get(), RooArgList(*var),cuts1.c_str(),o.cutRange) ||
2099-
! fillHistogram(hist2.get(), RooArgList(*var),cuts2.c_str(),o.cutRange)) {
2100-
coutE(Plotting) << ClassName() << "::" << GetName()
2101-
<< ":plotEffOn: createHistogram() failed" << std::endl;
2102-
return nullptr;
2103-
}
2104-
2105-
// convert this histogram to a RooHist object on the heap
2106-
RooHist *graph= new RooHist(*hist1,*hist2,0,1,o.etype,o.xErrorSize,true);
2107-
graph->setYAxisLabel(Form("Efficiency of %s=%s", effCat.GetName(), effCat.lookupName(1).c_str()));
2079+
graph->setYAxisLabel(("Efficiency of " + catName + "=" + effCat.lookupName(1)).c_str());
21082080

21092081
// initialize the frame's normalization setup, if necessary
21102082
frame->updateNormVars(_vars);
@@ -2113,12 +2085,12 @@ RooPlot* RooAbsData::plotEffOn(RooPlot* frame, const RooAbsCategoryLValue& effCa
21132085
if (o.histName) {
21142086
graph->SetName(o.histName) ;
21152087
} else {
2116-
std::string hname(Form("h_%s_Eff[%s]",GetName(),effCat.GetName())) ;
2117-
if (o.cutRange && strlen(o.cutRange)>0) {
2118-
hname += Form("_CutRange[%s]",o.cutRange);
2088+
std::string hname = "h_" + std::string{GetName()} + "_Eff[" + catName + "]";
2089+
if (o.cutRange && strlen(o.cutRange) > 0) {
2090+
hname += "_CutRange[" + std::string{o.cutRange} + " ]";
21192091
}
21202092
if (o.cuts && strlen(o.cuts)>0) {
2121-
hname += Form("_Cut[%s]",o.cuts);
2093+
hname += "_Cut[" + std::string{o.cuts} + " ]";
21222094
}
21232095
graph->SetName(hname.c_str()) ;
21242096
}
@@ -2614,11 +2586,12 @@ TH2F *RooAbsData::createHistogram(const RooAbsRealLValue &var1, const RooAbsReal
26142586
}
26152587
}
26162588

2617-
const std::string histName = std::string{GetName()} + "_" + name + "_" + Form("%08x", counter++);
2589+
std::stringstream histName;
2590+
histName << GetName() << "_" << name << "_" << std::setw(8) << std::setfill('0') << std::hex << counter++;
26182591

26192592
// create the histogram
26202593
auto *histogram =
2621-
new TH2F(histName.c_str(), "Events", nx, var1.getMin(), var1.getMax(), ny, var2.getMin(), var2.getMax());
2594+
new TH2F(histName.str().c_str(), "Events", nx, var1.getMin(), var1.getMax(), ny, var2.getMin(), var2.getMax());
26222595
if (!histogram) {
26232596
coutE(DataHandling) << GetName() << "::createHistogram: unable to create a new histogram" << std::endl;
26242597
return nullptr;

‎roofit/roofitcore/src/RooAbsReal.cxx

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,7 @@ RooFit::OwningPtr<RooAbsReal> RooAbsReal::createIntObj(const RooArgSet& iset2, c
669669
if (!cacheParams.empty()) {
670670
cxcoutD(Caching) << "RooAbsReal::createIntObj(" << GetName() << ") INFO: constructing " << cacheParams.size()
671671
<< "-dim value cache for integral over " << iset2 << " as a function of " << cacheParams << " in range " << (rangeName?rangeName:"<none>") << std::endl ;
672-
std::string name = Form("%s_CACHE_[%s]",integral->GetName(),cacheParams.contentsString().c_str()) ;
672+
std::string name = std::string{integral->GetName()} + "_CACHE_[" + cacheParams.contentsString() + "]";
673673
auto cachedIntegral = std::make_unique<RooCachedReal>(name.c_str(),name.c_str(),*integral,cacheParams);
674674
cachedIntegral->setInterpolationOrder(2) ;
675675
cachedIntegral->addOwnedComponents(std::move(integral));
@@ -1943,9 +1943,9 @@ RooPlot* RooAbsReal::plotOn(RooPlot *frame, PlotOpt o) const
19431943

19441944
// Inform user about projections
19451945
if (!projectedVars.empty()) {
1946-
coutI(Plotting) << "RooAbsReal::plotOn(" << GetName() << ") plot on " << plotVar->GetName()
1947-
<< " integrates over variables " << projectedVars
1948-
<< (o.projectionRangeName?Form(" in range %s",o.projectionRangeName):"") << std::endl;
1946+
coutI(Plotting) << "RooAbsReal::plotOn(" << GetName() << ") plot on " << plotVar->GetName()
1947+
<< " integrates over variables " << projectedVars
1948+
<< (o.projectionRangeName ? (" in range " + std::string{o.projectionRangeName}) : "") << std::endl;
19491949
}
19501950
if (projDataNeededVars && !projDataNeededVars->empty()) {
19511951
coutI(Plotting) << "RooAbsReal::plotOn(" << GetName() << ") plot on " << plotVar->GetName()
@@ -2387,15 +2387,16 @@ RooPlot* RooAbsReal::plotAsymOn(RooPlot *frame, const RooAbsCategoryLValue& asym
23872387

23882388

23892389
// Set default name of curve
2390-
TString curveName(funcAsym.GetName()) ;
2390+
std::stringstream curveName;
2391+
curveName << funcAsym.GetName();
23912392
if (!sliceSet.empty()) {
2392-
curveName.Append(Form("_Slice[%s]",sliceSet.contentsString().c_str())) ;
2393+
curveName << "_Slice[" << sliceSet.contentsString() << "]";
23932394
}
23942395
if (o.curveNameSuffix) {
23952396
// Append any suffixes imported from RooAbsPdf::plotOn
2396-
curveName.Append(o.curveNameSuffix) ;
2397+
curveName << o.curveNameSuffix;
23972398
}
2398-
curve->SetName(curveName.Data()) ;
2399+
curve->SetName(curveName.str().c_str());
23992400

24002401
// add this new curve to the specified plot frame
24012402
frame->addPlotable(curve, o.drawOptions);

‎roofit/roofitcore/src/RooProdPdf.cxx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,14 +1059,14 @@ void RooProdPdf::rearrangeProduct(RooProdPdf::CacheElem& cache) const
10591059

10601060
RooAddition* tmpadd = static_cast<RooAddition*>(parg) ;
10611061

1062-
RooCustomizer cust(*tmpadd->list1().first(),Form("blah_%s",iter->c_str())) ;
1062+
RooCustomizer cust(*tmpadd->list1().first(), ("blah_" + *iter).c_str());
10631063
cust.replaceArg(*ratio,*specializedRatio) ;
10641064
partCust = cust.build() ;
10651065

10661066
} else {
1067-
RooCustomizer cust(*parg,Form("blah_%s",iter->c_str())) ;
1068-
cust.replaceArg(*ratio,*specializedRatio) ;
1069-
partCust = cust.build() ;
1067+
RooCustomizer cust(*parg, ("blah_" + *iter).c_str());
1068+
cust.replaceArg(*ratio, *specializedRatio);
1069+
partCust = cust.build();
10701070
}
10711071

10721072
// Print customized denominator
@@ -1139,7 +1139,7 @@ void RooProdPdf::rearrangeProduct(RooProdPdf::CacheElem& cache) const
11391139
return ;
11401140
}
11411141

1142-
string name = Form("%s_numerator",GetName()) ;
1142+
string name = std::string{GetName()} + "_numerator";
11431143
// WVE FIX THIS (2)
11441144

11451145
std::unique_ptr<RooAbsReal> numerator = std::make_unique<RooProduct>(name.c_str(),name.c_str(),nomList) ;

‎roofit/roofitcore/src/RooStudyManager.cxx

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,6 @@ repeated applications of generate-and-fit operations on a workspace
4141
#include "TROOT.h"
4242
#include "TSystem.h"
4343

44-
using std::string, std::endl, std::ios, std::list, std::ofstream;
45-
46-
47-
4844
////////////////////////////////////////////////////////////////////////////////
4945

5046
RooStudyManager::RooStudyManager(RooWorkspace &w) : _pkg(new RooStudyPackage(w)) {}
@@ -62,10 +58,10 @@ RooStudyManager::RooStudyManager(RooWorkspace &w, RooAbsStudy &study) : _pkg(new
6258

6359
RooStudyManager::RooStudyManager(const char* studyPackFileName)
6460
{
65-
string pwd = gDirectory->GetName() ;
66-
std::unique_ptr<TFile> f{TFile::Open(studyPackFileName, "READ")};
67-
_pkg = dynamic_cast<RooStudyPackage*>(f->Get("studypack")) ;
68-
gDirectory->cd(Form("%s:",pwd.c_str())) ;
61+
std::string pwd = gDirectory->GetName();
62+
std::unique_ptr<TFile> f{TFile::Open(studyPackFileName, "READ")};
63+
_pkg = dynamic_cast<RooStudyPackage *>(f->Get("studypack"));
64+
gDirectory->cd((pwd + ":").c_str());
6965
}
7066

7167

@@ -99,7 +95,7 @@ void RooStudyManager::prepareBatchInput(const char* studyName, Int_t nExpPerJob,
9995
if (unifiedInput) {
10096

10197
// Write header of driver script
102-
ofstream bdr(Form("study_driver_%s.sh",studyName)) ;
98+
std::ofstream bdr(Form("study_driver_%s.sh", studyName));
10399
bdr << "#!/bin/sh" << std::endl
104100
<< Form("if [ ! -f study_data_%s.root ] ; then",studyName) << std::endl
105101
<< "uudecode <<EOR" << std::endl ;
@@ -109,7 +105,7 @@ void RooStudyManager::prepareBatchInput(const char* studyName, Int_t nExpPerJob,
109105
gSystem->Exec(Form("cat study_data_%s.root | uuencode -m study_data_%s.root >> study_driver_%s.sh",studyName,studyName,studyName)) ;
110106

111107
// Write remainder of driver script
112-
ofstream bdr2 (Form("study_driver_%s.sh",studyName),ios::app) ;
108+
std::ofstream bdr2(Form("study_driver_%s.sh", studyName), std::ios::app);
113109
bdr2 << "EOR" << std::endl
114110
<< "fi" << std::endl
115111
<< "root -l -b <<EOR" << std::endl
@@ -124,15 +120,16 @@ void RooStudyManager::prepareBatchInput(const char* studyName, Int_t nExpPerJob,
124120

125121
} else {
126122

127-
ofstream bdr(Form("study_driver_%s.sh",studyName)) ;
128-
bdr << "#!/bin/sh" << std::endl
129-
<< "root -l -b <<EOR" << std::endl
130-
<< Form("RooStudyPackage::processFile(\"%s\",%d) ;",studyName,nExpPerJob) << std::endl
131-
<< ".q" << std::endl
132-
<< "EOR" << std::endl ;
123+
std::ofstream bdr(Form("study_driver_%s.sh", studyName));
124+
bdr << "#!/bin/sh" << std::endl
125+
<< "root -l -b <<EOR" << std::endl
126+
<< Form("RooStudyPackage::processFile(\"%s\",%d) ;", studyName, nExpPerJob) << std::endl
127+
<< ".q" << std::endl
128+
<< "EOR" << std::endl;
133129

134-
coutI(DataHandling) << "RooStudyManager::prepareBatchInput batch driver file is '" << Form("study_driver_%s.sh",studyName) << "," << std::endl
135-
<< " input data file is " << Form("study_data_%s.root",studyName) << std::endl ;
130+
coutI(DataHandling) << "RooStudyManager::prepareBatchInput batch driver file is '"
131+
<< Form("study_driver_%s.sh", studyName) << "," << std::endl
132+
<< " input data file is " << Form("study_data_%s.root", studyName) << std::endl;
136133

137134
}
138135
}
@@ -144,32 +141,32 @@ void RooStudyManager::prepareBatchInput(const char* studyName, Int_t nExpPerJob,
144141

145142
void RooStudyManager::processBatchOutput(const char* filePat)
146143
{
147-
list<string> flist ;
148-
expandWildCardSpec(filePat,flist) ;
144+
std::list<std::string> flist;
145+
expandWildCardSpec(filePat, flist);
149146

150-
TList olist ;
147+
TList olist;
151148

152-
for (list<string>::iterator iter = flist.begin() ; iter!=flist.end() ; ++iter) {
153-
coutP(DataHandling) << "RooStudyManager::processBatchOutput() now reading file " << *iter << std::endl ;
154-
TFile f(iter->c_str()) ;
149+
for (auto iter = flist.begin(); iter != flist.end(); ++iter) {
150+
coutP(DataHandling) << "RooStudyManager::processBatchOutput() now reading file " << *iter << std::endl;
151+
TFile f(iter->c_str());
155152

156-
for(auto * key : static_range_cast<TKey*>(*f.GetListOfKeys())) {
157-
TObject * obj = f.Get(key->GetName()) ;
158-
olist.Add(obj->Clone(obj->GetName())) ;
159-
}
160-
}
153+
for (auto *key : static_range_cast<TKey *>(*f.GetListOfKeys())) {
154+
TObject *obj = f.Get(key->GetName());
155+
olist.Add(obj->Clone(obj->GetName()));
156+
}
157+
}
161158
aggregateData(&olist) ;
162159
olist.Delete() ;
163160
}
164161

165162

166163
////////////////////////////////////////////////////////////////////////////////
167164

168-
void RooStudyManager::aggregateData(TList* olist)
165+
void RooStudyManager::aggregateData(TList *olist)
169166
{
170-
for (list<RooAbsStudy*>::iterator iter=_pkg->studies().begin() ; iter!=_pkg->studies().end() ; ++iter) {
171-
(*iter)->aggregateSummaryOutput(olist) ;
172-
}
167+
for (auto study : _pkg->studies()) {
168+
study->aggregateSummaryOutput(olist);
169+
}
173170
}
174171

175172

@@ -178,7 +175,7 @@ void RooStudyManager::aggregateData(TList* olist)
178175
////////////////////////////////////////////////////////////////////////////////
179176
/// case with one single file
180177

181-
void RooStudyManager::expandWildCardSpec(const char* name, list<string>& result)
178+
void RooStudyManager::expandWildCardSpec(const char *name, std::list<std::string> &result)
182179
{
183180
if (!TString(name).MaybeWildcard()) {
184181
result.push_back(name) ;

0 commit comments

Comments
 (0)