[v626][RF] Backports of RooFit PRs to v6-26-00-patches: Part 19

roofit/roofit/inc/RooLagrangianMorphFunc.h

@@ -106,7 +106,7 @@ class RooLagrangianMorphFunc : public RooAbsReal {
    RooLagrangianMorphFunc(const char *name, const char *title, const Config &config);
    RooLagrangianMorphFunc(const RooLagrangianMorphFunc &other, const char *newName);
 
-   virtual ~RooLagrangianMorphFunc();
+   ~RooLagrangianMorphFunc() override;
 
    std::list<double> *binBoundaries(RooAbsRealLValue & /*obs*/, double /*xlo*/, double /*xhi*/) const override;
    std::list<double> *plotSamplingHint(RooAbsRealLValue & /*obs*/, double /*xlo*/, double /*xhi*/) const override;
@@ -230,24 +230,23 @@ class RooLagrangianMorphFunc : public RooAbsReal {
    void setScale(double val);
    double getScale();
 
-   int nSamples() const { return this->_config.folderNames.size(); }
+   int nSamples() const { return _config.folderNames.size(); }
 
    RooRealSumFunc *getFunc() const;
    std::unique_ptr<RooWrapperPdf> createPdf() const;
 
    RooAbsPdf::ExtendMode extendMode() const;
-   Double_t expectedEvents(const RooArgSet *nset) const;
-   Double_t expectedEvents(const RooArgSet &nset) const;
-   Double_t expectedEvents() const;
-   Bool_t selfNormalized() const { return true; }
+   double expectedEvents(const RooArgSet *nset) const;
+   double expectedEvents(const RooArgSet &nset) const;
+   double expectedEvents() const;
+   bool selfNormalized() const { return true; }
 
    void readParameters(TDirectory *f);
    void collectInputs(TDirectory *f);
 
    static std::unique_ptr<RooRatio> makeRatio(const char *name, const char *title, RooArgList &nr, RooArgList &dr);
 
 private:
-
    mutable RooObjCacheManager _cacheMgr; //! The cache manager
    double _scale = 1.0;
    std::map<std::string, int> _sampleMap;

roofit/roofitcore/inc/RooAbsRealLValue.h

@@ -157,6 +157,8 @@ class RooAbsRealLValue : public RooAbsReal, public RooAbsLValue {
   static TH1* createHistogram(const char *name, RooArgList &vars, const char *tAxisLabel, Double_t* xlo, Double_t* xhi, Int_t* nBins) ;
   static TH1* createHistogram(const char *name, RooArgList &vars, const char *tAxisLabel, const RooAbsBinning** bins) ;
 
+  RooAbsReal* createIntegral(const RooArgSet& iset, const RooArgSet* nset=nullptr, const RooNumIntConfig* cfg=nullptr, const char* rangeName=nullptr) const;
+
 protected:
 
   virtual void setValFast(Double_t value) { setVal(value) ; }

roofit/roofitcore/inc/RooArgProxy.h

@@ -48,9 +48,19 @@ class RooArgProxy : public TNamed, public RooAbsProxy  {
   /// Returns the owner of this proxy.
   RooAbsArg* owner() const { return _owner; }
 
+  inline Bool_t isValueServer() const { 
+    // Returns true of contents is value server of owner
+    return _valueServer ; 
+  }
+  inline Bool_t isShapeServer() const { 
+    // Returns true if contents is shape server of owner
+    return _shapeServer ; 
+  }
+
 protected:
 
-  friend class RooSimultaneous ;
+  friend class RooRealIntegral;
+
   RooAbsArg* _owner ;       // Pointer to owner of proxy
   RooAbsArg* _arg ;         // Pointer to content of proxy
 
@@ -59,16 +69,6 @@ class RooArgProxy : public TNamed, public RooAbsProxy  {
   Bool_t _isFund ;          // If true proxy contains an lvalue
   Bool_t _ownArg ;          // If true proxy owns contents
 
-  friend class RooAbsArg ;
-
-  inline Bool_t isValueServer() const { 
-    // Returns true of contents is value server of owner
-    return _valueServer ; 
-  }
-  inline Bool_t isShapeServer() const { 
-    // Returns true if contents is shape server of owner
-    return _shapeServer ; 
-  }
   virtual Bool_t changePointer(const RooAbsCollection& newServerSet, Bool_t nameChange=kFALSE, Bool_t factoryInitMode=kFALSE) ;
 
   virtual void changeDataSet(const RooArgSet* newNormSet) ;

roofit/roofitcore/inc/RooEffProd.h

@@ -13,13 +13,13 @@
 #define ROO_EFF_PROD
 
 #include "RooAbsPdf.h"
-#include "RooAbsReal.h"
 #include "RooRealProxy.h"
 #include "RooObjCacheManager.h"
 
 class RooEffProd: public RooAbsPdf {
 public:
   // Constructors, assignment etc
+  RooEffProd() {}
   RooEffProd(const char *name, const char *title, RooAbsPdf& pdf, RooAbsReal& efficiency);
   RooEffProd(const RooEffProd& other, const char* name=0);
 

roofit/roofitcore/res/RooFit/BatchModeDataHelpers.h

@@ -30,10 +30,9 @@ class TNamed;
 namespace RooFit {
 namespace BatchModeDataHelpers {
 
-std::map<RooFit::Detail::DataKey, RooSpan<const double>> getDataSpans(RooAbsData const &data,
-                                                                      std::string_view rangeName,
-                                                                      RooAbsCategory const *indexCat,
-                                                                      std::stack<std::vector<double>> &buffers);
+std::map<RooFit::Detail::DataKey, RooSpan<const double>>
+getDataSpans(RooAbsData const &data, std::string_view rangeName, RooAbsCategory const *indexCat,
+             std::stack<std::vector<double>> &buffers, bool skipZeroWeights);
 
 } // namespace BatchModeDataHelpers
 } // namespace RooFit

roofit/roofitcore/res/RooFitDriver.h

@@ -50,7 +50,7 @@ class RooFitDriver {
                 RooFit::BatchModeOption batchMode = RooFit::BatchModeOption::Cpu);
 
    void setData(RooAbsData const &data, std::string_view rangeName = "",
-                RooAbsCategory const *indexCatForSplitting = nullptr);
+                RooAbsCategory const *indexCatForSplitting = nullptr, bool skipZeroWeights = false);
    void setData(DataSpansMap const &dataSpans);
 
    ~RooFitDriver();

roofit/roofitcore/src/BatchModeDataHelpers.cxx

@@ -22,8 +22,8 @@
 
 namespace {
 
-void splitByCategory(std::map<RooFit::Detail::DataKey, RooSpan<const double>> &dataSpans, RooAbsCategory const &category,
-                     std::stack<std::vector<double>> &buffers)
+void splitByCategory(std::map<RooFit::Detail::DataKey, RooSpan<const double>> &dataSpans,
+                     RooAbsCategory const &category, std::stack<std::vector<double>> &buffers)
 {
    std::stack<std::vector<double>> oldBuffers;
    std::swap(buffers, oldBuffers);
@@ -71,28 +71,33 @@ void splitByCategory(std::map<RooFit::Detail::DataKey, RooSpan<const double>> &d
 } // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
-// Extract all content from a RooFit datasets as a map of spans.
-// Spans with the weights and squared weights will be also stored in the map,
-// keyed with the names `_weight` and the `_weight_sumW2`. If the dataset is
-// unweighted, these weight spans will only contain the single value `1.0`.
-// Entries with zero weight will be skipped.
-//
-// \return A `std::map` with spans keyed to name pointers.
-// \param[in] data The input dataset.
-// \param[in] rangeName Select only entries from the data in a given range
-//            (empty string for no range).
-// \param[in] indexCat If not `nullptr`, each span is spit up by this category,
-//            with the new names prefixed by the category component name
-//            surrounded by underscores. For example, if you have a category
-//            with `signal` and `control` samples, the span for a variable `x`
-//            will be split in two spans `_signal_x` and `_control_x`.
-// \param[in] buffers Pass here an empty stack of `double` vectors, which will
-//            be used as memory for the data if the memory in the dataset
-//            object can't be used directly (e.g. because you used the range
-//            selection or the splitting by categories).
+/// Extract all content from a RooFit datasets as a map of spans.
+/// Spans with the weights and squared weights will be also stored in the map,
+/// keyed with the names `_weight` and the `_weight_sumW2`. If the dataset is
+/// unweighted, these weight spans will only contain the single value `1.0`.
+/// Entries with zero weight will be skipped.
+///
+/// \return A `std::map` with spans keyed to name pointers.
+/// \param[in] data The input dataset.
+/// \param[in] rangeName Select only entries from the data in a given range
+///            (empty string for no range).
+/// \param[in] indexCat If not `nullptr`, each span is spit up by this category,
+///            with the new names prefixed by the category component name
+///            surrounded by underscores. For example, if you have a category
+///            with `signal` and `control` samples, the span for a variable `x`
+///            will be split in two spans `_signal_x` and `_control_x`.
+/// \param[in] buffers Pass here an empty stack of `double` vectors, which will
+///            be used as memory for the data if the memory in the dataset
+///            object can't be used directly (e.g. because you used the range
+///            selection or the splitting by categories).
+/// \param[in] skipZeroWeights Skip entries with zero weight when filling the
+///            data spans. Be very careful with enabling it, because the user
+///            might not expect that the batch results are not aligned with the
+///            original dataset anymore!
 std::map<RooFit::Detail::DataKey, RooSpan<const double>>
 RooFit::BatchModeDataHelpers::getDataSpans(RooAbsData const &data, std::string_view rangeName,
-                                           RooAbsCategory const *indexCat, std::stack<std::vector<double>> &buffers)
+                                           RooAbsCategory const *indexCat, std::stack<std::vector<double>> &buffers,
+                                           bool skipZeroWeights)
 {
    std::map<RooFit::Detail::DataKey, RooSpan<const double>> dataSpans; // output variable
 
@@ -137,7 +142,7 @@ RooFit::BatchModeDataHelpers::getDataSpans(RooAbsData const &data, std::string_v
          buffer.reserve(nEvents);
          bufferSumW2.reserve(nEvents);
          for (std::size_t i = 0; i < nEvents; ++i) {
-            if (weight[i] != 0) {
+            if (!skipZeroWeights || weight[i] != 0) {
                buffer.push_back(weight[i]);
                bufferSumW2.push_back(weightSumW2[i]);
                ++nNonZeroWeight;

roofit/roofitcore/src/BatchModeHelpers.cxx

@@ -34,30 +34,32 @@ using ROOT::Experimental::RooNLLVarNew;
 
 namespace {
 
-std::unique_ptr<RooAbsArg> prepareSimultaneousModelForBatchMode(RooSimultaneous &simPdf, RooArgSet &observables,
-                                                                bool isExtended, std::string const &rangeName,
-                                                                bool doOffset)
+std::unique_ptr<RooAbsArg> createSimultaneousNLL(std::unique_ptr<RooSimultaneous> &&simPdf, RooArgSet &observables,
+                                                 bool isExtended, std::string const &rangeName, bool doOffset)
 {
    // Prepare the NLLTerms for each component
    RooArgList nllTerms;
    RooArgSet newObservables;
-   for (auto const &catItem : simPdf.indexCat()) {
+   for (auto const &catItem : simPdf->indexCat()) {
       auto const &catName = catItem.first;
-      if (RooAbsPdf *pdf = simPdf.getPdf(catName.c_str())) {
-         auto nllName = std::string("nll_") + pdf->GetName();
-         auto *nll =
-            new RooNLLVarNew(nllName.c_str(), nllName.c_str(), *pdf, observables, isExtended, rangeName, doOffset);
+      if (RooAbsPdf *pdf = simPdf->getPdf(catName.c_str())) {
+         auto name = std::string("nll_") + pdf->GetName();
+         auto nll = std::make_unique<RooNLLVarNew>(name.c_str(), name.c_str(), *pdf, observables, isExtended, rangeName,
+                                                   doOffset);
          // Rename the observables and weights
          newObservables.add(nll->prefixObservableAndWeightNames(std::string("_") + catName + "_"));
-         nllTerms.add(*nll);
+         nllTerms.addOwned(std::move(nll));
       }
    }
 
    observables.clear();
    observables.add(newObservables);
 
    // Time to sum the NLLs
-   return std::make_unique<RooAddition>("mynll", "mynll", nllTerms, true);
+   auto nll = std::make_unique<RooAddition>("mynll", "mynll", nllTerms);
+   nll->addOwnedComponents(std::move(simPdf));
+   nll->addOwnedComponents(std::move(nllTerms));
+   return nll;
 }
 
 } // namespace
@@ -97,12 +99,16 @@ RooFit::BatchModeHelpers::createNLL(RooAbsPdf &pdf, RooAbsData &data, std::uniqu
 
    RooArgList nllTerms;
 
+   RooAbsCategory const *indexCatForSplitting = nullptr;
    if (auto simPdf = dynamic_cast<RooSimultaneous *>(&finalPdf)) {
-      auto *simPdfClone = static_cast<RooSimultaneous *>(simPdf->cloneTree());
+      RooArgSet parameters;
+      finalPdf.getParameters(data.get(), parameters);
+      std::unique_ptr<RooSimultaneous> simPdfClone{static_cast<RooSimultaneous *>(simPdf->cloneTree())};
+      indexCatForSplitting = &simPdfClone->indexCat();
+      simPdfClone->recursiveRedirectServers(parameters);
       simPdfClone->wrapPdfsInBinSamplingPdfs(data, integrateOverBinsPrecision);
       // Warning! This mutates "observables"
-      nllTerms.addOwned(
-         prepareSimultaneousModelForBatchMode(*simPdfClone, observables, isExtended, rangeName, doOffset));
+      nllTerms.addOwned(createSimultaneousNLL(std::move(simPdfClone), observables, isExtended, rangeName, doOffset));
    } else {
       nllTerms.addOwned(std::make_unique<RooNLLVarNew>("RooNLLVarNew", "RooNLLVarNew", finalPdf, observables,
                                                        isExtended, rangeName, doOffset));
@@ -116,16 +122,8 @@ RooFit::BatchModeHelpers::createNLL(RooAbsPdf &pdf, RooAbsData &data, std::uniqu
    nll->addOwnedComponents(std::move(binSamplingPdfs));
    nll->addOwnedComponents(std::move(nllTerms));
 
-   if (auto simPdf = dynamic_cast<RooSimultaneous *>(&finalPdf)) {
-      RooArgSet parameters;
-      pdf.getParameters(data.get(), parameters);
-      nll->recursiveRedirectServers(parameters);
-      driver = std::make_unique<RooFitDriver>(*nll, observables, batchMode);
-      driver->setData(data, rangeName, &simPdf->indexCat());
-   } else {
-      driver = std::make_unique<RooFitDriver>(*nll, observables, batchMode);
-      driver->setData(data, rangeName);
-   }
+   driver = std::make_unique<RooFitDriver>(*nll, observables, batchMode);
+   driver->setData(data, rangeName, indexCatForSplitting, /*skipZeroWeights=*/true);
 
    // Set the fitrange attribute so that RooPlot can automatically plot the fitting range by default
    if (!rangeName.empty()) {
@@ -195,10 +193,9 @@ namespace {
 
 class RooAbsRealWrapper final : public RooAbsReal {
 public:
-   RooAbsRealWrapper() {}
-   RooAbsRealWrapper(RooFitDriver &driver, RooArgSet const &observables, bool ownsDriver)
-      : RooAbsReal{"RooFitDriverWrapper", "RooFitDriverWrapper"}, _driver{&driver},
-        _topNode("topNode", "top node", this, _driver->topNode()), _ownsDriver{ownsDriver}
+   RooAbsRealWrapper(std::unique_ptr<RooFitDriver> driver, RooArgSet const &observables)
+      : RooAbsReal{"RooFitDriverWrapper", "RooFitDriverWrapper"}, _driver{std::move(driver)},
+        _topNode("topNode", "top node", this, _driver->topNode())
    {
       _driver->topNode().getParameters(&observables, _parameters, true);
    }
@@ -209,20 +206,16 @@ class RooAbsRealWrapper final : public RooAbsReal {
    {
    }
 
-   ~RooAbsRealWrapper() override
-   {
-      if (_ownsDriver)
-         delete _driver;
-   }
-
    TObject *clone(const char *newname) const override { return new RooAbsRealWrapper(*this, newname); }
 
    double defaultErrorLevel() const override { return _driver->topNode().defaultErrorLevel(); }
 
-   bool getParameters(const RooArgSet * /*observables*/, RooArgSet &outputSet,
-                      bool /*stripDisconnected=true*/) const override
+   bool getParameters(const RooArgSet *observables, RooArgSet &outputSet, bool /*stripDisconnected*/) const override
    {
       outputSet.add(_parameters);
+      if (observables) {
+         outputSet.remove(*observables);
+      }
       return false;
    }
 
@@ -237,10 +230,9 @@ class RooAbsRealWrapper final : public RooAbsReal {
    double evaluate() const override { return _driver ? _driver->getVal() : 0.0; }
 
 private:
-   RooFitDriver *_driver = nullptr;
+   std::shared_ptr<RooFitDriver> _driver;
    RooRealProxy _topNode;
    RooArgSet _parameters;
-   bool _ownsDriver;
 };
 
 } // namespace
@@ -251,5 +243,5 @@ std::unique_ptr<RooAbsReal>
 RooFit::BatchModeHelpers::makeDriverAbsRealWrapper(std::unique_ptr<ROOT::Experimental::RooFitDriver> driver,
                                                    RooArgSet const &observables)
 {
-   return std::unique_ptr<RooAbsReal>{new RooAbsRealWrapper{*driver.release(), observables, true}};
+   return std::make_unique<RooAbsRealWrapper>(std::move(driver), observables);
 }

roofit/roofitcore/src/NormalizationHelpers.cxx

@@ -171,8 +171,9 @@ void treeNodeServerListAndNormSets(const RooAbsArg &arg, RooAbsCollection &list,
          }
 
          auto differentSet = arg.fillNormSetForServer(normSet, *server);
-         if (differentSet)
+         if (differentSet) {
             differentSet->sort();
+         }
 
          auto &serverNormSet = differentSet ? *differentSet : normSet;
 
@@ -220,14 +221,17 @@ std::vector<std::unique_ptr<RooAbsArg>> unfoldIntegrals(RooAbsArg const &topNode
    treeNodeServerListAndNormSets(topNode, nodes, normSetSorted, normSets, checker);
 
    // Clean normsets of the variables that the arg does not depend on
-   // std::unordered_map<std::pair<RooAbsArg const*,RooAbsArg const*>,bool> dependsResults;
    for (auto &item : normSets) {
       if (!item.second || item.second->empty())
          continue;
       auto actualNormSet = new RooArgSet{};
       for (auto *narg : *item.second) {
          if (checker.dependsOn(item.first, narg))
-            actualNormSet->add(*narg);
+            // Add the arg from the actual node list in the computation graph.
+            // Like this, we don't accidentally add internal variable clones
+            // that the client args returned. Looking this up is fast because
+            // of the name pointer hash map optimization.
+            actualNormSet->add(*nodes.find(*narg));
       }
       delete item.second;
       item.second = actualNormSet;

roofit/roofitcore/src/RooAbsRealLValue.cxx

@@ -1059,3 +1059,14 @@ Bool_t RooAbsRealLValue::isJacobianOK(const RooArgSet&) const
   // always returns true (i.e. jacobian is constant)
   return kTRUE ; 
 }
+
+
+RooAbsReal* RooAbsRealLValue::createIntegral(const RooArgSet&, const RooArgSet*, const RooNumIntConfig*, const char*) const
+{
+  std::stringstream errStream;
+  errStream << "Attempting to integrate the " << ClassName() << " \"" << GetName()
+            << "\", but integrating a RooAbsRealLValue is not allowed!";
+  const std::string errString = errStream.str();
+  coutE(InputArguments) << errString << std::endl;
+  throw std::runtime_error(errString);
+}

roofit/roofitcore/src/RooEffProd.cxx

@@ -24,8 +24,6 @@
 #include "RooFit.h"
 #include "RooEffProd.h"
 #include "RooEffGenContext.h"
-#include "RooNameReg.h"
-#include "RooRealVar.h"
 
 
 ////////////////////////////////////////////////////////////////////////////////