Skip to content

Commit ead7ca2

Browse files
guitargeekguitargeek
committed
[RF] Factor out adding of RooRealIntegral servers into helper function
This is done to have a clearer picture of what is going on.
1 parent 3fa7244 commit ead7ca2

File tree

1 file changed

+123
-101
lines changed

1 file changed

+123
-101
lines changed

‎roofit/roofitcore/src/RooRealIntegral.cxx‎

Lines changed: 123 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,128 @@ using namespace std;
5757
ClassImp(RooRealIntegral);
5858

5959

60+
namespace {
61+
62+
void addValueAndShapeServers(RooAbsArg &integral, RooAbsReal const &function, RooArgSet const &depList,
63+
RooArgSet const &intDepList, const char *rangeName, RooArgSet &anIntOKDepList)
64+
{
65+
66+
for (const auto arg : function.servers()) {
67+
68+
// Dependent or parameter?
69+
if (!arg->dependsOnValue(intDepList)) {
70+
71+
if (function.dependsOnValue(*arg)) {
72+
integral.addServer(*arg, true, false);
73+
}
74+
75+
continue;
76+
77+
} else {
78+
79+
// Add final dependents of arg as shape servers
80+
81+
// Skip arg if it is neither value or shape server
82+
if (!arg->isValueServer(function) && !arg->isShapeServer(function)) {
83+
continue;
84+
}
85+
86+
// We keep track separately of all leaves and the leaves that are only
87+
// value servers to `arg`, which in this code branch is always a value
88+
// server of the original function. With the additional value-only server
89+
// list, we can quicky check if a given leaf is also a value server to
90+
// the top-level `function`, because if and only if `leaf` is a value
91+
// server of `arg` is it also a value server of `function`. The expensive
92+
// calls to `function.dependsOnValue(*leaf)` that were used before are
93+
// avoided like this.
94+
RooArgSet argLeafServers;
95+
RooArgSet argLeafValueServers;
96+
arg->treeNodeServerList(&argLeafServers, nullptr, false, true, /*valueOnly=*/false, false);
97+
arg->treeNodeServerList(&argLeafValueServers, nullptr, false, true, /*valueOnly=*/true, false);
98+
99+
for (const auto leaf : argLeafServers) {
100+
101+
if (depList.find(leaf->GetName()) && argLeafValueServers.contains(*leaf)) {
102+
103+
auto *leaflv = dynamic_cast<RooAbsRealLValue *>(leaf);
104+
if (leaflv && leaflv->getBinning(rangeName).isParameterized()) {
105+
oocxcoutD(&function, Integration)
106+
<< function.GetName() << " : Observable " << leaf->GetName()
107+
<< " has parameterized binning, add value dependence of boundary objects rather than shape of leaf"
108+
<< endl;
109+
if (leaflv->getBinning(rangeName).lowBoundFunc()) {
110+
integral.addServer(*leaflv->getBinning(rangeName).lowBoundFunc(), true, false);
111+
}
112+
if (leaflv->getBinning(rangeName).highBoundFunc()) {
113+
integral.addServer(*leaflv->getBinning(rangeName).highBoundFunc(), true, false);
114+
}
115+
} else {
116+
oocxcoutD(&function, Integration) << function.GetName() << ": Adding observable " << leaf->GetName()
117+
<< " of server " << arg->GetName() << " as shape dependent" << endl;
118+
integral.addServer(*leaf, false, true);
119+
}
120+
} else if (!depList.find(leaf->GetName())) {
121+
122+
if (argLeafValueServers.contains(*leaf)) {
123+
oocxcoutD(&function, Integration) << function.GetName() << ": Adding parameter " << leaf->GetName()
124+
<< " of server " << arg->GetName() << " as value dependent" << endl;
125+
integral.addServer(*leaf, true, false);
126+
} else {
127+
oocxcoutD(&function, Integration) << function.GetName() << ": Adding parameter " << leaf->GetName()
128+
<< " of server " << arg->GetName() << " as shape dependent" << endl;
129+
integral.addServer(*leaf, false, true);
130+
}
131+
}
132+
}
133+
}
134+
135+
// If this dependent arg is self-normalized, stop here
136+
// if (function.selfNormalized()) continue ;
137+
138+
bool depOK(false);
139+
// Check for integratable AbsRealLValue
140+
141+
if (arg->isDerived()) {
142+
RooAbsRealLValue *realArgLV = dynamic_cast<RooAbsRealLValue *>(arg);
143+
RooAbsCategoryLValue *catArgLV = dynamic_cast<RooAbsCategoryLValue *>(arg);
144+
if ((realArgLV && intDepList.find(realArgLV->GetName()) && (realArgLV->isJacobianOK(intDepList) != 0)) ||
145+
catArgLV) {
146+
147+
// Derived LValue with valid jacobian
148+
depOK = true;
149+
150+
// Now, check for overlaps
151+
bool overlapOK = true;
152+
for (const auto otherArg : function.servers()) {
153+
// skip comparison with self
154+
if (arg == otherArg)
155+
continue;
156+
if (otherArg->IsA() == RooConstVar::Class())
157+
continue;
158+
if (arg->overlaps(*otherArg, true)) {
159+
}
160+
}
161+
// coverity[DEADCODE]
162+
if (!overlapOK)
163+
depOK = false;
164+
}
165+
} else {
166+
// Fundamental types are always OK
167+
depOK = true;
168+
}
169+
170+
// Add server to list of dependents that are OK for analytical integration
171+
if (depOK) {
172+
anIntOKDepList.add(*arg, true);
173+
oocxcoutI(&function, Integration) << function.GetName() << ": Observable " << arg->GetName()
174+
<< " is suitable for analytical integration (if supported by p.d.f)" << endl;
175+
}
176+
}
177+
}
178+
179+
} // namespace
180+
181+
60182
Int_t RooRealIntegral::_cacheAllNDim(2) ;
61183

62184

@@ -296,108 +418,8 @@ RooRealIntegral::RooRealIntegral(const char *name, const char *title,
296418
// Add all parameters/dependents as value/shape servers *
297419
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
298420

299-
for (const auto arg : function.servers()) {
300-
301-
// Dependent or parameter?
302-
if (!arg->dependsOnValue(intDepList)) {
421+
addValueAndShapeServers(*this, function, depList, intDepList, rangeName, anIntOKDepList);
303422

304-
if (function.dependsOnValue(*arg)) {
305-
addServer(*arg,true,false) ;
306-
}
307-
308-
continue ;
309-
310-
} else {
311-
312-
// Add final dependents of arg as shape servers
313-
314-
// We keep track separately of all leaves and the leaves that are only
315-
// value servers to `arg`, which in this code branch is always a value
316-
// server of the original function. With the additional value-only server
317-
// list, we can quicky check if a given leaf is also a value server to
318-
// the top-level `function`, because if and only if `leaf` is a value
319-
// server of `arg` is it also a value server of `function`. The expensive
320-
// calls to `function.dependsOnValue(*leaf)` that were used before are
321-
// avoided like this.
322-
RooArgSet argLeafServers ;
323-
RooArgSet argLeafValueServers ;
324-
arg->treeNodeServerList(&argLeafServers,nullptr,false,true,/*valueOnly=*/false,false) ;
325-
arg->treeNodeServerList(&argLeafValueServers,nullptr,false,true,/*valueOnly=*/true,false) ;
326-
327-
// Skip arg if it is neither value or shape server
328-
if (!arg->isValueServer(function) && !arg->isShapeServer(function)) {
329-
continue ;
330-
}
331-
332-
for (const auto leaf : argLeafServers) {
333-
334-
if (depList.find(leaf->GetName()) && argLeafValueServers.contains(*leaf)) {
335-
336-
RooAbsRealLValue* leaflv = dynamic_cast<RooAbsRealLValue*>(leaf) ;
337-
if (leaflv && leaflv->getBinning(rangeName).isParameterized()) {
338-
oocxcoutD(&function,Integration) << function.GetName() << " : Observable " << leaf->GetName() << " has parameterized binning, add value dependence of boundary objects rather than shape of leaf" << endl ;
339-
if (leaflv->getBinning(rangeName).lowBoundFunc()) {
340-
addServer(*leaflv->getBinning(rangeName).lowBoundFunc(),true,false) ;
341-
}
342-
if(leaflv->getBinning(rangeName).highBoundFunc()) {
343-
addServer(*leaflv->getBinning(rangeName).highBoundFunc(),true,false) ;
344-
}
345-
} else {
346-
oocxcoutD(&function,Integration) << function.GetName() << ": Adding observable " << leaf->GetName() << " of server "
347-
<< arg->GetName() << " as shape dependent" << endl ;
348-
addServer(*leaf,false,true) ;
349-
}
350-
} else if (!depList.find(leaf->GetName())) {
351-
352-
if (argLeafValueServers.contains(*leaf)) {
353-
oocxcoutD(&function,Integration) << function.GetName() << ": Adding parameter " << leaf->GetName() << " of server " << arg->GetName() << " as value dependent" << endl ;
354-
addServer(*leaf,true,false) ;
355-
} else {
356-
oocxcoutD(&function,Integration) << function.GetName() << ": Adding parameter " << leaf->GetName() << " of server " << arg->GetName() << " as shape dependent" << endl ;
357-
addServer(*leaf,false,true) ;
358-
}
359-
}
360-
}
361-
}
362-
363-
// If this dependent arg is self-normalized, stop here
364-
//if (function.selfNormalized()) continue ;
365-
366-
bool depOK(false) ;
367-
// Check for integratable AbsRealLValue
368-
369-
if (arg->isDerived()) {
370-
RooAbsRealLValue *realArgLV = dynamic_cast<RooAbsRealLValue*>(arg) ;
371-
RooAbsCategoryLValue *catArgLV = dynamic_cast<RooAbsCategoryLValue*>(arg) ;
372-
if ((realArgLV && intDepList.find(realArgLV->GetName()) && (realArgLV->isJacobianOK(intDepList)!=0)) || catArgLV) {
373-
374-
// Derived LValue with valid jacobian
375-
depOK = true ;
376-
377-
// Now, check for overlaps
378-
bool overlapOK = true ;
379-
for (const auto otherArg : function.servers()) {
380-
// skip comparison with self
381-
if (arg==otherArg) continue ;
382-
if (otherArg->IsA()==RooConstVar::Class()) continue ;
383-
if (arg->overlaps(*otherArg,true)) {
384-
}
385-
}
386-
// coverity[DEADCODE]
387-
if (!overlapOK) depOK=false ;
388-
389-
}
390-
} else {
391-
// Fundamental types are always OK
392-
depOK = true ;
393-
}
394-
395-
// Add server to list of dependents that are OK for analytical integration
396-
if (depOK) {
397-
anIntOKDepList.add(*arg,true) ;
398-
oocxcoutI(&function,Integration) << function.GetName() << ": Observable " << arg->GetName() << " is suitable for analytical integration (if supported by p.d.f)" << endl ;
399-
}
400-
}
401423
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
402424
// * E) interact with function to make list of objects actually integrated analytically *
403425
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *

0 commit comments

Comments
 (0)