Skip to content

Commit a3c4cd6

Browse files
guitargeekanigamova
authored andcommitted
Don't hide categories when creating the RooMinimizer
Indeed, the RooMinimizer can't deal with floating category variables. But instead of hiding the categories when creating the RooMinimizer, one can also set them to constant and the minimizer will not complain too. This solution is less hacky than hooking into `getParameters()` of the CachingSimNLL. This was originally introduced by @gpetrucc in #574 Quoting from that PR: ``` For the dirty flag propagation to the NLL when the categories are changed, and option to avoid reporting the categories in getParameters() to avoid complaints from RooMinimizer ``` So indeed, hiding the categories was supposed to address these warnings, which are also gone when the categories are just set to constant.
1 parent 8bca53e commit a3c4cd6

File tree

3 files changed

+22
-12
lines changed

3 files changed

+22
-12
lines changed

interface/CachingNLL.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,6 @@ class CachingSimNLL : public RooAbsReal {
182182
static void forceUnoptimizedConstraints() { optimizeContraints_ = false; }
183183
void setChannelMasks(RooArgList const& args);
184184
void setAnalyticBarlowBeeston(bool flag);
185-
void setHideRooCategories(bool flag) { hideRooCategories_ = flag; }
186185
void setHideConstants(bool flag) { hideConstants_ = flag; }
187186
void setMaskConstraints(bool flag) ;
188187
void setMaskNonDiscreteChannels(bool mask) ;
@@ -195,7 +194,6 @@ class CachingSimNLL : public RooAbsReal {
195194
const RooAbsData *dataOriginal_;
196195
const RooArgSet *nuis_;
197196
RooSetProxy params_, catParams_;
198-
bool hideRooCategories_ = false;
199197
bool hideConstants_ = false;
200198
RooArgSet piecesForCloning_;
201199
std::unique_ptr<RooSimultaneous> factorizedPdf_;

src/CachingNLL.cc

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -838,7 +838,6 @@ cacheutils::CachingSimNLL::CachingSimNLL(const CachingSimNLL &other, const char
838838
nuis_(other.nuis_),
839839
params_("params","parameters",this),
840840
catParams_("catParams","Category parameters",this),
841-
hideRooCategories_(other.hideRooCategories_),
842841
hideConstants_(other.hideConstants_),
843842
internalMasks_(other.internalMasks_),
844843
maskConstraints_(other.maskConstraints_),
@@ -1240,10 +1239,10 @@ cacheutils::CachingSimNLL::getParameters(const RooArgSet* depList, Bool_t stripD
12401239
RooArgSet *ret;
12411240
if (internalMasks_.empty()) {
12421241
ret = new RooArgSet(params_);
1243-
if (!hideRooCategories_) ret->add(catParams_);
1242+
ret->add(catParams_);
12441243
} else {
12451244
ret = new RooArgSet(activeParameters_);
1246-
if (!hideRooCategories_) ret->add(activeCatParameters_);
1245+
ret->add(activeCatParameters_);
12471246
}
12481247
if (hideConstants_) RooStats::RemoveConstantParameters(ret);
12491248
return ret;
@@ -1255,10 +1254,10 @@ bool cacheutils::CachingSimNLL::getParameters(const RooArgSet* depList,
12551254
{
12561255
if (internalMasks_.empty()) {
12571256
outputSet.add(params_);
1258-
if (!hideRooCategories_) outputSet.add(catParams_);
1257+
outputSet.add(catParams_);
12591258
} else {
12601259
outputSet.add(activeParameters_);
1261-
if (!hideRooCategories_) outputSet.add(activeCatParameters_);
1260+
outputSet.add(activeCatParameters_);
12621261
}
12631262
if (hideConstants_) RooStats::RemoveConstantParameters(&outputSet);
12641263
return true;

src/CascadeMinimizer.cc

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <RooStats/RooStatsUtils.h>
1515

1616
#include <iomanip>
17+
#include <memory>
1718

1819
boost::program_options::options_description CascadeMinimizer::options_("Cascade Minimizer options");
1920
std::vector<CascadeMinimizer::Algo> CascadeMinimizer::fallbacks_;
@@ -53,11 +54,23 @@ CascadeMinimizer::CascadeMinimizer(RooAbsReal &nll, Mode mode, RooRealVar *poi)
5354
}
5455

5556
void CascadeMinimizer::remakeMinimizer() {
56-
cacheutils::CachingSimNLL *simnll = dynamic_cast<cacheutils::CachingSimNLL *>(&nll_);
57-
if (simnll) simnll->setHideRooCategories(true);
58-
minimizer_.reset(); // avoid two copies in memory
59-
minimizer_.reset(new RooMinimizer(nll_));
60-
if (simnll) simnll->setHideRooCategories(false);
57+
std::unique_ptr<RooArgSet> nllParams(nll_.getParameters((const RooArgSet*)nullptr));
58+
RooArgSet changedSet;
59+
60+
// The RooMinimizer can't deal with floating categories, so we need to set them constant at least while creating the minimizer.
61+
for (RooAbsArg *arg : *nllParams) {
62+
auto *cat = dynamic_cast<RooCategory *>(arg);
63+
if (cat && !cat->isConstant()) {
64+
cat->setConstant(true);
65+
changedSet.add(*cat);
66+
}
67+
}
68+
69+
minimizer_.reset(); // avoid two copies in memory
70+
minimizer_ = std::make_unique<RooMinimizer>(nll_);
71+
72+
// Change categories back to how they were
73+
utils::setAllConstant(changedSet, /*constant=*/false);
6174
}
6275

6376
bool CascadeMinimizer::freezeDiscParams(const bool freeze)

0 commit comments

Comments
 (0)