diff --git a/docs/part3/nonstandard.md b/docs/part3/nonstandard.md index d54c9dc9eab..dcd94dd7359 100644 --- a/docs/part3/nonstandard.md +++ b/docs/part3/nonstandard.md @@ -568,8 +568,6 @@ As expected, the curve obtained by allowing the `pdf_index` to float (labelled " In general, the performance of Combine can be improved when using the discrete profiling method by including the option `--X-rtd MINIMIZER_freezeDisassociatedParams`. This will stop parameters not associated to the current PDF from floating in the fits. Additionally, you can include the following options: -- `--X-rtd MINIMIZER_multiMin_hideConstants`: hide the constant terms in the likelihood when recreating the minimizer -- `--X-rtd MINIMIZER_multiMin_maskConstraints`: hide the constraint terms during the discrete minimization process - `--X-rtd MINIMIZER_multiMin_maskChannels=` mask the channels that are not needed from the NLL: - ` 1`: keeps unmasked all channels that are participating in the discrete minimization. - ` 2`: keeps unmasked only the channel whose index is being scanned at the moment. diff --git a/interface/CachingNLL.h b/interface/CachingNLL.h index 8145881a7a7..e69ec83b3ff 100644 --- a/interface/CachingNLL.h +++ b/interface/CachingNLL.h @@ -178,9 +178,6 @@ class CachingSimNLL : public RooAbsReal { static void forceUnoptimizedConstraints() { optimizeContraints_ = false; } void setChannelMasks(RooArgList const& args); void setAnalyticBarlowBeeston(bool flag); - void setHideRooCategories(bool flag) { hideRooCategories_ = flag; } - void setHideConstants(bool flag) { hideConstants_ = flag; } - void setMaskConstraints(bool flag) ; void setMaskNonDiscreteChannels(bool mask) ; friend class CachingAddNLL; // trap this call, since we don't care about propagating it to the sub-components @@ -191,8 +188,6 @@ class CachingSimNLL : public RooAbsReal { const RooAbsData *dataOriginal_; const RooArgSet *nuis_; RooSetProxy params_, catParams_; - bool hideRooCategories_ = false; - bool hideConstants_ = false; RooArgSet piecesForCloning_; std::unique_ptr factorizedPdf_; std::vector constrainPdfs_; @@ -212,7 +207,6 @@ class CachingSimNLL : public RooAbsReal { std::vector constrainZeroPointsFastPoisson_; std::vector channelMasks_; std::vector internalMasks_; - bool maskConstraints_ = false; RooArgSet activeParameters_, activeCatParameters_; double maskingOffset_ = 0; // offset to ensure that interal or constraint masking doesn't change NLL value double maskingOffsetZero_ = 0; // and associated zero point diff --git a/src/CachingNLL.cc b/src/CachingNLL.cc index 8b2578392e8..67008ad3d96 100644 --- a/src/CachingNLL.cc +++ b/src/CachingNLL.cc @@ -847,10 +847,7 @@ cacheutils::CachingSimNLL::CachingSimNLL(const CachingSimNLL &other, const char nuis_(other.nuis_), params_("params","parameters",this), catParams_("catParams","Category parameters",this), - hideRooCategories_(other.hideRooCategories_), - hideConstants_(other.hideConstants_), internalMasks_(other.internalMasks_), - maskConstraints_(other.maskConstraints_), maskingOffset_(other.maskingOffset_), maskingOffsetZero_(other.maskingOffsetZero_) { @@ -1049,7 +1046,7 @@ cacheutils::CachingSimNLL::evaluate() const ret += nllval; } } - if (!maskConstraints_ && (!constrainPdfs_.empty() || !constrainPdfsFast_.empty() || !constrainPdfsFastPoisson_.empty() || !constrainPdfGroups_.empty())) { + if (!constrainPdfs_.empty() || !constrainPdfsFast_.empty() || !constrainPdfsFastPoisson_.empty() || !constrainPdfGroups_.empty()) { DefaultAccumulator ret2 = 0; /// ============= GENERIC CONSTRAINTS ========= std::vector::const_iterator itz = constrainZeroPoints_.begin(); @@ -1253,12 +1250,11 @@ cacheutils::CachingSimNLL::getParameters(const RooArgSet* depList, Bool_t stripD RooArgSet *ret; if (internalMasks_.empty()) { ret = new RooArgSet(params_); - if (!hideRooCategories_) ret->add(catParams_); + ret->add(catParams_); } else { ret = new RooArgSet(activeParameters_); - if (!hideRooCategories_) ret->add(activeCatParameters_); + ret->add(activeCatParameters_); } - if (hideConstants_) RooStats::RemoveConstantParameters(ret); return ret; } #else @@ -1268,25 +1264,15 @@ bool cacheutils::CachingSimNLL::getParameters(const RooArgSet* depList, { if (internalMasks_.empty()) { outputSet.add(params_); - if (!hideRooCategories_) outputSet.add(catParams_); + outputSet.add(catParams_); } else { outputSet.add(activeParameters_); - if (!hideRooCategories_) outputSet.add(activeCatParameters_); + outputSet.add(activeCatParameters_); } - if (hideConstants_) RooStats::RemoveConstantParameters(&outputSet); return true; } #endif -void cacheutils::CachingSimNLL::setMaskConstraints(bool flag) { - double nllBefore = evaluate(); - maskConstraints_ = flag; - double nllAfter = evaluate(); - maskingOffset_ += (nllBefore - nllAfter); - //printf("CachingSimNLL: setMaskConstraints(%d): nll before %.12g, nll after %.12g (diff %.12g), new maskingOffset %.12g, check = %.12g\n", - // int(flag), nllBefore, nllAfter, (nllBefore-nllAfter), maskingOffset_, evaluate() - nllBefore); -} - void cacheutils::CachingSimNLL::setMaskNonDiscreteChannels(bool mask) { double nllBefore = evaluate(); internalMasks_.clear(); // reset diff --git a/src/CascadeMinimizer.cc b/src/CascadeMinimizer.cc index 76f1992cd26..f453d5f5a6c 100644 --- a/src/CascadeMinimizer.cc +++ b/src/CascadeMinimizer.cc @@ -53,11 +53,23 @@ CascadeMinimizer::CascadeMinimizer(RooAbsReal &nll, Mode mode, RooRealVar *poi) } void CascadeMinimizer::remakeMinimizer() { - cacheutils::CachingSimNLL *simnll = dynamic_cast(&nll_); - if (simnll) simnll->setHideRooCategories(true); - minimizer_.reset(); // avoid two copies in memory - minimizer_.reset(new RooMinimizer(nll_)); - if (simnll) simnll->setHideRooCategories(false); + std::unique_ptr nllParams(nll_.getParameters((const RooArgSet*)0)); + RooArgSet changedSet; + + // The RooMinimizer can't deal with floating categories, so we need to set them constant at least while creating the minimizer. + for (RooAbsArg *arg : *nllParams) { + auto cat = dynamic_cast(arg); + if (cat && !cat->isConstant()) { + cat->setConstant(true); + changedSet.add(*cat); + } + } + + minimizer_.reset(); // avoid two copies in memory + minimizer_.reset(new RooMinimizer(nll_)); + + // Change categories back to how they were + utils::setAllConstant(changedSet, false); } bool CascadeMinimizer::freezeDiscParams(const bool freeze) @@ -484,8 +496,6 @@ bool CascadeMinimizer::minimize(int verbose, bool cascade) bool CascadeMinimizer::multipleMinimize(const RooArgSet &reallyCleanParameters, bool& ret, double& minimumNLL, int verbose, bool cascade,int mode, std::vector >&contributingIndeces){ static bool freezeDisassParams = runtimedef::get(std::string("MINIMIZER_freezeDisassociatedParams")); - static bool hideConstants = freezeDisassParams && runtimedef::get(std::string("MINIMIZER_multiMin_hideConstants")); - static bool maskConstraints = freezeDisassParams && runtimedef::get(std::string("MINIMIZER_multiMin_maskConstraints")); static int maskChannels = freezeDisassParams ? runtimedef::get(std::string("MINIMIZER_multiMin_maskChannels")) : 0; cacheutils::CachingSimNLL *simnll = dynamic_cast(&nll_); @@ -552,11 +562,6 @@ bool CascadeMinimizer::multipleMinimize(const RooArgSet &reallyCleanParameters, if (maskChannels && simnll) { simnll->setMaskNonDiscreteChannels(true); } - if (hideConstants && simnll) { - simnll->setHideConstants(true); - if (maskConstraints) simnll->setMaskConstraints(true); - minimizer_.reset(); // will be recreated when needed by whoever needs it - } std::vector > myCombos; @@ -694,11 +699,6 @@ bool CascadeMinimizer::multipleMinimize(const RooArgSet &reallyCleanParameters, if (maskChannels && simnll) { simnll->setMaskNonDiscreteChannels(false); } - if (hideConstants && simnll) { - simnll->setHideConstants(false); - if (maskConstraints) simnll->setMaskConstraints(false); - minimizer_.reset(); // will be recreated when needed by whoever needs it - } return newDiscreteMinimum;