Remove optional hiding of constants and constraints

Originally added in 2019 by @gpetrucc in
#576.

I quote that PR:

```
It is possible that already setting maskChannels=2 and
SIMNLL_GROUPCONSTRAINTS=N make the gain from hideConstant and
maskConstraints redundant in practice but I haven't tested since I
developed them in the order they're listed here.
```

I think this is indeed the case. Maybe it would be good to check if
removing these options indeed has relevant effect on performance,
because reducing the number of these RooFit hacks helps to converge
between Combine and upstream RooFit.

Author: guitargeek

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 <span style="font-variant:small-caps;">Combine</span> 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=<choice>` mask the channels that are not needed from the NLL:
   - `<choice> 1`: keeps unmasked all channels that are participating in the discrete minimization.
   - `<choice> 2`: keeps unmasked only the channel whose index is being scanned at the moment.

interface/CachingNLL.h

@@ -182,8 +182,6 @@ class CachingSimNLL  : public RooAbsReal {
         static void forceUnoptimizedConstraints() { optimizeContraints_ = false; }
         void setChannelMasks(RooArgList const& args);
         void setAnalyticBarlowBeeston(bool 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
@@ -194,7 +192,6 @@ class CachingSimNLL  : public RooAbsReal {
         const RooAbsData  *dataOriginal_;
         const RooArgSet   *nuis_;
         RooSetProxy        params_, catParams_;
-        bool hideConstants_ = false;
         RooArgSet piecesForCloning_;
         std::unique_ptr<RooSimultaneous>  factorizedPdf_;
         std::vector<RooAbsPdf *>        constrainPdfs_;
@@ -214,7 +211,6 @@ class CachingSimNLL  : public RooAbsReal {
         std::vector<double> constrainZeroPointsFastPoisson_;
         std::vector<RooAbsReal*> channelMasks_;
         std::vector<bool>        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

src/CachingNLL.cc

@@ -838,9 +838,7 @@ cacheutils::CachingSimNLL::CachingSimNLL(const CachingSimNLL &other, const char
     nuis_(other.nuis_),
     params_("params","parameters",this),
     catParams_("catParams","Category parameters",this),
-    hideConstants_(other.hideConstants_),
     internalMasks_(other.internalMasks_),
-    maskConstraints_(other.maskConstraints_),
     maskingOffset_(other.maskingOffset_),
     maskingOffsetZero_(other.maskingOffsetZero_)
 {
@@ -1047,7 +1045,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<double> ret2 = 0;
         /// ============= GENERIC CONSTRAINTS  =========
         for (std::size_t i = 0; i < constrainPdfs_.size(); ++i) {
@@ -1244,7 +1242,6 @@ cacheutils::CachingSimNLL::getParameters(const RooArgSet* depList, Bool_t stripD
         ret = new RooArgSet(activeParameters_); 
         ret->add(activeCatParameters_);
     }
-    if (hideConstants_) RooStats::RemoveConstantParameters(ret);
     return ret;
 }
 #else
@@ -1259,20 +1256,10 @@ bool cacheutils::CachingSimNLL::getParameters(const RooArgSet* depList,
         outputSet.add(activeParameters_);
         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

src/CascadeMinimizer.cc

@@ -515,8 +515,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<std::vector<bool> >&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<cacheutils::CachingSimNLL *>(&nll_);
 
@@ -583,11 +581,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<std::vector<int> > myCombos;
 
@@ -725,11 +718,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;