Skip to content

Commit 9604027

Browse files
guitargeekguitargeek
committed
[RF] Avoid creation of many dummy RooRealVars in RooAddPdf evaluation
The cache element of the RooAddPdf is unnecessarily large, as it contains several RooArgLists that contain many dummy RooRealVars with value one. The footprint of the cache object is reduced by using `std::vectors` instead, and fill null pointers instead of the dummy RooRealVars.
1 parent 7363049 commit 9604027

File tree

4 files changed

+72
-72
lines changed

4 files changed

+72
-72
lines changed

‎roofit/roofitcore/src/RooAddHelpers.cxx‎

Lines changed: 34 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R
2121
const RooArgSet *nset, const RooArgSet *iset, const char *rangeName, bool projectCoefs,
2222
RooArgSet const &refCoefNorm, TNamed const *refCoefRangeName, int verboseEval)
2323
{
24+
_suppNormList.reserve(pdfList.size());
25+
2426
// *** PART 1 : Create supplemental normalization list ***
2527

2628
// Retrieve the combined set of dependents of this PDF ;
@@ -50,27 +52,20 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R
5052

5153
std::unique_ptr<RooAbsReal> snorm;
5254
auto name = std::string(addPdf.GetName()) + "_" + pdf->GetName() + "_SupNorm";
53-
_needSupNorm = false;
5455
if (!supNSet.empty()) {
5556
snorm = std::make_unique<RooRealIntegral>(name.c_str(), "Supplemental normalization integral",
5657
RooRealConstant::value(1.0), supNSet);
5758
oocxcoutD(&addPdf, Caching) << addPdf.ClassName() << " " << addPdf.GetName()
5859
<< " making supplemental normalization set " << supNSet << " for pdf component "
5960
<< pdf->GetName() << std::endl;
60-
_needSupNorm = true;
61-
} else {
62-
snorm = std::make_unique<RooRealVar>(name.c_str(), "Unit Supplemental normalization integral", 1.0);
6361
}
64-
_suppNormList.addOwned(std::move(snorm));
62+
_suppNormList.emplace_back(std::move(snorm));
6563
}
6664

6765
if (verboseEval > 1) {
6866
oocxcoutD(&addPdf, Caching) << addPdf.ClassName() << "::syncSuppNormList(" << addPdf.GetName()
6967
<< ") synching supplemental normalization list for norm"
7068
<< (nset ? *nset : RooArgSet()) << std::endl;
71-
if oodologD (&addPdf, Caching) {
72-
_suppNormList.Print("v");
73-
}
7469
}
7570

7671
// *** PART 2 : Create projection coefficients ***
@@ -125,17 +120,11 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R
125120
oocxcoutD(&addPdf, Caching) << addPdf.ClassName() << "(" << addPdf.GetName() << ")::getPC nset2(" << nset2
126121
<< ")!=_refCoefNorm(" << refCoefNorm << ") --> pdfProj = " << pdfProj->GetName()
127122
<< std::endl;
128-
} else {
129-
auto name = std::string(addPdf.GetName()) + "_" + thePdf->GetName() + "_ProjectNorm";
130-
pdfProj = std::make_unique<RooRealVar>(name.c_str(), "Unit Projection normalization integral", 1.0);
131-
oocxcoutD(&addPdf, Caching) << addPdf.ClassName() << "(" << addPdf.GetName() << ")::getPC nset2(" << nset2
132-
<< ")==_refCoefNorm(" << refCoefNorm << ") --> pdfProj = " << pdfProj->GetName()
133-
<< std::endl;
123+
oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName()
124+
<< ") PP = " << pdfProj->GetName() << std::endl;
134125
}
135126

136-
oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName()
137-
<< ") PP = " << pdfProj->GetName() << std::endl;
138-
_projList.addOwned(std::move(pdfProj));
127+
_projList.emplace_back(std::move(pdfProj));
139128

140129
// Calculation optional supplemental normalization term
141130
RooArgSet supNormSet(refCoefNorm);
@@ -147,13 +136,10 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R
147136
if (!supNormSet.empty() && !nset2.equals(refCoefNorm)) {
148137
snorm = std::make_unique<RooRealIntegral>(name.c_str(), "Projection Supplemental normalization integral",
149138
RooRealConstant::value(1.0), supNormSet);
150-
} else {
151-
snorm =
152-
std::make_unique<RooRealVar>(name.c_str(), "Unit Projection Supplemental normalization integral", 1.0);
139+
oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName()
140+
<< ") SN = " << snorm->GetName() << std::endl;
153141
}
154-
oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName()
155-
<< ") SN = " << snorm->GetName() << std::endl;
156-
_suppProjList.addOwned(std::move(snorm));
142+
_suppProjList.emplace_back(std::move(snorm));
157143

158144
// Calculate reference range adjusted projection integral
159145
std::unique_ptr<RooAbsReal> rangeProj1;
@@ -189,15 +175,10 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R
189175
std::unique_ptr<RooAbsReal>{thePdf->createIntegral(tmp, tmp, RooNameReg::str(refCoefRangeName))};
190176

191177
// rangeProj1->setOperMode(operMode()) ;
192-
193-
} else {
194-
195-
auto theName = std::string(addPdf.GetName()) + "_" + thePdf->GetName() + "_RangeNorm1";
196-
rangeProj1 = std::make_unique<RooRealVar>(theName.c_str(), "Unit range normalization integral", 1.0);
178+
oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName()
179+
<< ") R1 = " << rangeProj1->GetName() << std::endl;
197180
}
198-
oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName()
199-
<< ") R1 = " << rangeProj1->GetName() << std::endl;
200-
_refRangeProjList.addOwned(std::move(rangeProj1));
181+
_refRangeProjList.emplace_back(std::move(rangeProj1));
201182

202183
// Calculate range adjusted projection integral
203184
std::unique_ptr<RooAbsReal> rangeProj2;
@@ -214,15 +195,10 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R
214195
RooArgSet tmp;
215196
thePdf->getObservables(&refCoefNorm, tmp);
216197
rangeProj2 = std::unique_ptr<RooAbsReal>{thePdf->createIntegral(tmp, tmp, addPdf.normRange())};
217-
218-
} else {
219-
220-
auto theName = std::string(addPdf.GetName()) + "_" + thePdf->GetName() + "_RangeNorm2";
221-
rangeProj2 = std::make_unique<RooRealVar>(theName.c_str(), "Unit range normalization integral", 1.0);
198+
oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName()
199+
<< ") R2 = " << rangeProj2->GetName() << std::endl;
222200
}
223-
oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName()
224-
<< ") R2 = " << rangeProj2->GetName() << std::endl;
225-
_rangeProjList.addOwned(std::move(rangeProj2));
201+
_rangeProjList.emplace_back(std::move(rangeProj2));
226202
}
227203
}
228204
}
@@ -233,10 +209,23 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R
233209
RooArgList AddCacheElem::containedArgs(Action)
234210
{
235211
RooArgList allNodes;
236-
allNodes.add(_projList);
237-
allNodes.add(_suppProjList);
238-
allNodes.add(_refRangeProjList);
239-
allNodes.add(_rangeProjList);
212+
// need to iterate manually because _suppProjList can contain nullptr
213+
for (auto const &arg : _projList) {
214+
if (arg)
215+
allNodes.add(*arg);
216+
}
217+
for (auto const &arg : _suppProjList) {
218+
if (arg)
219+
allNodes.add(*arg);
220+
}
221+
for (auto const &arg : _refRangeProjList) {
222+
if (arg)
223+
allNodes.add(*arg);
224+
}
225+
for (auto const &arg : _rangeProjList) {
226+
if (arg)
227+
allNodes.add(*arg);
228+
}
240229

241230
return allNodes;
242231
}
@@ -323,7 +312,7 @@ void RooAddHelpers::updateCoefficients(RooAbsPdf const &addPdf, RooArgList const
323312
}
324313

325314
// Stop here if not projection is required or needed
326-
if ((!projectCoefs && !addPdf.normRange()) || cache._projList.empty()) {
315+
if ((!projectCoefs && !addPdf.normRange()) || !cache.doProjection()) {
327316
return;
328317
}
329318

@@ -333,15 +322,7 @@ void RooAddHelpers::updateCoefficients(RooAbsPdf const &addPdf, RooArgList const
333322
RooAbsReal::GlobalSelectComponentRAII compRAII(true);
334323

335324
for (std::size_t i = 0; i < pdfList.size(); i++) {
336-
337-
RooAbsReal *pp = ((RooAbsReal *)cache._projList.at(i));
338-
RooAbsReal *sn = ((RooAbsReal *)cache._suppProjList.at(i));
339-
RooAbsReal *r1 = ((RooAbsReal *)cache._refRangeProjList.at(i));
340-
RooAbsReal *r2 = ((RooAbsReal *)cache._rangeProjList.at(i));
341-
342-
double proj = pp->getVal() / sn->getVal() * (r2->getVal() / r1->getVal());
343-
344-
coefCache[i] *= proj;
325+
coefCache[i] *= cache.projVal(i) / cache.projSuppNormVal(i) * cache.rangeProjScaleFactor(i);
345326
coefSum += coefCache[i];
346327
}
347328
}

‎roofit/roofitcore/src/RooAddHelpers.h‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
#include <RooAbsCacheElement.h>
1515
#include <RooArgList.h>
16+
#include <RooAbsReal.h>
1617

1718
class RooAbsPdf;
1819
class RooArgSet;
@@ -25,13 +26,37 @@ class AddCacheElem : public RooAbsCacheElement {
2526

2627
RooArgList containedArgs(Action) override;
2728

28-
RooArgList _suppNormList; ///< Supplemental normalization list
29-
bool _needSupNorm; ///< Does the above list contain any non-unit entries?
29+
inline double suppNormVal(std::size_t idx) const { return _suppNormList[idx] ? _suppNormList[idx]->getVal() : 1.0; }
3030

31-
RooArgList _projList; ///< Projection integrals to be multiplied with coefficients
32-
RooArgList _suppProjList; ///< Projection integrals to multiply with coefficients for supplemental normalization
33-
RooArgList _refRangeProjList; ///< Range integrals to be multiplied with coefficients (reference range)
34-
RooArgList _rangeProjList; ///< Range integrals to be multiplied with coefficients (target range)
31+
inline bool doProjection() const { return !_projList.empty(); }
32+
33+
inline double projVal(std::size_t idx) const { return _projList[idx] ? _projList[idx]->getVal() : 1.0; }
34+
35+
inline double projSuppNormVal(std::size_t idx) const
36+
{
37+
return _suppProjList[idx] ? _suppProjList[idx]->getVal() : 1.0;
38+
}
39+
40+
inline double rangeProjScaleFactor(std::size_t idx) const { return rangeProjVal(idx) / refRangeProjVal(idx); }
41+
42+
private:
43+
inline double refRangeProjVal(std::size_t idx) const
44+
{
45+
return _refRangeProjList[idx] ? _refRangeProjList[idx]->getVal() : 1.0;
46+
}
47+
48+
inline double rangeProjVal(std::size_t idx) const
49+
{
50+
return _rangeProjList[idx] ? _rangeProjList[idx]->getVal() : 1.0;
51+
}
52+
53+
using OwningArgVector = std::vector<std::unique_ptr<RooAbsReal>>;
54+
55+
OwningArgVector _suppNormList; ///< Supplemental normalization list
56+
OwningArgVector _projList; ///< Projection integrals to be multiplied with coefficients
57+
OwningArgVector _suppProjList; ///< Projection integrals to multiply with coefficients for supplemental normalization
58+
OwningArgVector _refRangeProjList; ///< Range integrals to be multiplied with coefficients (reference range)
59+
OwningArgVector _rangeProjList; ///< Range integrals to be multiplied with coefficients (target range)
3560
};
3661

3762
class RooAddHelpers {

‎roofit/roofitcore/src/RooAddModel.cxx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ double RooAddModel::evaluate() const
364364
auto pdf = static_cast<RooAbsPdf*>(obj);
365365

366366
if (_coefCache[i]!=0.) {
367-
snormVal = nset ? ((RooAbsReal*)cache->_suppNormList.at(i))->getVal() : 1.0 ;
367+
snormVal = nset ? cache->suppNormVal(i) : 1.0 ;
368368
double pdfVal = pdf->getVal(nset) ;
369369
// double pdfNorm = pdf->getNorm(nset) ;
370370
if (pdf->isSelectedComp()) {
@@ -516,7 +516,7 @@ double RooAddModel::analyticalIntegralWN(Int_t code, const RooArgSet* normSet, c
516516
for (const auto obj : *compIntList) {
517517
auto pdfInt = static_cast<const RooAbsReal*>(obj);
518518
if (_coefCache[i]!=0.) {
519-
snormVal = nset ? ((RooAbsReal*)pcache->_suppNormList.at(i))->getVal() : 1.0 ;
519+
snormVal = nset ? pcache->suppNormVal(i) : 1.0 ;
520520
double intVal = pdfInt->getVal(nset) ;
521521
value += intVal*_coefCache[i]/snormVal ;
522522
cxcoutD(Eval) << "RooAddModel::evaluate(" << GetName() << ") value += ["

‎roofit/roofitcore/src/RooAddPdf.cxx‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -463,9 +463,7 @@ double RooAddPdf::getValV(const RooArgSet* normSet) const
463463
for (unsigned int i=0; i < _pdfList.size(); ++i) {
464464
const auto& pdf = static_cast<RooAbsPdf&>(_pdfList[i]);
465465
double snormVal = 1.;
466-
if (cache->_needSupNorm) {
467-
snormVal = ((RooAbsReal*)cache->_suppNormList.at(i))->getVal();
468-
}
466+
snormVal = cache->suppNormVal(i);
469467

470468
double pdfVal = pdf.getVal(nset);
471469
if (pdf.isSelectedComp()) {
@@ -492,8 +490,7 @@ void RooAddPdf::computeBatch(cudaStream_t* stream, double* output, size_t nEvent
492490
if (pdf->isSelectedComp())
493491
{
494492
pdfs.push_back(dataMap.at(pdf));
495-
coefs.push_back(_coefCache[pdfNo] / (cache->_needSupNorm ?
496-
static_cast<RooAbsReal*>(cache->_suppNormList.at(pdfNo))->getVal() : 1) );
493+
coefs.push_back(_coefCache[pdfNo] / cache->suppNormVal(pdfNo) );
497494
}
498495
}
499496
auto dispatch = stream ? RooBatchCompute::dispatchCUDA : RooBatchCompute::dispatchCPU;
@@ -631,12 +628,11 @@ double RooAddPdf::analyticalIntegralWN(Int_t code, const RooArgSet* normSet, con
631628
double snormVal ;
632629

633630
//cout << "ROP::aIWN updateCoefCache with rangeName = " << (rangeName?rangeName:"<null>") << endl ;
634-
RooArgList* snormSet = (!cache->_suppNormList.empty()) ? &cache->_suppNormList : nullptr ;
635631
for (std::size_t i = 0; i < _pdfList.size(); ++i ) {
636632
auto pdf = static_cast<const RooAbsPdf*>(_pdfList.at(i));
637633

638634
if (_coefCache[i]) {
639-
snormVal = snormSet ? ((RooAbsReal*) cache->_suppNormList.at(i))->getVal() : 1.0 ;
635+
snormVal = cache->suppNormVal(i);
640636

641637
// WVE swap this?
642638
double val = pdf->analyticalIntegralWN(subCode[i],normSet,rangeName) ;
@@ -664,14 +660,12 @@ double RooAddPdf::expectedEvents(const RooArgSet* nset) const
664660
AddCacheElem& cache = *getProjCache(nset) ;
665661
updateCoefficients(cache, nset);
666662

667-
if (!cache._rangeProjList.empty()) {
663+
if (cache.doProjection()) {
668664

669665
for (std::size_t i = 0; i < _pdfList.size(); ++i) {
670-
auto const& r1 = static_cast<RooAbsReal&>(cache._refRangeProjList[i]);
671-
auto const& r2 = static_cast<RooAbsReal&>(cache._rangeProjList[i]);
672666
double ncomp = _allExtendable ? static_cast<RooAbsPdf&>(_pdfList[i]).expectedEvents(nset)
673667
: static_cast<RooAbsReal&>(_coefList[i]).getVal(nset);
674-
expectedTotal += (r2.getVal()/r1.getVal()) * ncomp ;
668+
expectedTotal += cache.rangeProjScaleFactor(i) * ncomp ;
675669

676670
}
677671

0 commit comments

Comments
 (0)