[RF] RooPolyFunc improvements

roofit/roofitcore/inc/RooBinnedGenContext.h

@@ -47,7 +47,7 @@ class RooBinnedGenContext : public RooAbsGenContext {
   RooBinnedGenContext(const RooBinnedGenContext& other) ;
 
   const RooArgSet* _vars ;
-  RooArgSet* _pdfSet ;          ///<  Set owned all nodes of internal clone of p.d.f
+  RooArgSet _pdfSet ;          ///<  Set owned all nodes of internal clone of p.d.f
   RooAbsPdf *_pdf ;             ///<  Pointer to cloned p.d.f
   RooDataHist* _hist ;          ///< Histogram
   bool _expectedData ;        ///< Asimov?

roofit/roofitcore/inc/RooEffGenContext.h

@@ -33,12 +33,12 @@ class RooEffGenContext : public RooAbsGenContext {
   void generateEvent(RooArgSet &theEvent, Int_t remaining) override;
 
 private:
-   RooArgSet* _cloneSet;           ///< Internal clone of p.d.f.
+   RooArgSet _cloneSet;            ///< Internal clone of p.d.f.
    RooAbsReal* _eff;               ///< Pointer to efficiency function
    RooAbsGenContext* _generator;   ///< Generator context for p.d.f
-   RooArgSet* _vars;               ///< Vars to generate
+   RooArgSet _vars;               ///< Vars to generate
    double _maxEff;                 ///< Maximum of efficiency in vars
 
-   ClassDefOverride(RooEffGenContext, 1) // Context for generating a dataset from a PDF
+   ClassDefOverride(RooEffGenContext, 0); // Context for generating a dataset from a PDF
 };
 #endif

roofit/roofitcore/inc/RooPolyFunc.h

@@ -17,6 +17,7 @@
 #define RooFit_RooFit_RooPolyFunc_h
 
 #include "RooAbsReal.h"
+#include "RooRealVar.h"
 #include "RooListProxy.h"
 
 #include <vector>
@@ -36,18 +37,25 @@ class RooPolyFunc : public RooAbsReal {
    TObject *clone(const char *newname) const override { return new RooPolyFunc(*this, newname); }
 
    std::string asString() const;
+   inline const RooArgList &variables() const { return _vars; }
+   inline const std::vector<std::unique_ptr<RooListProxy>> &terms() const { return _terms; }
+   inline RooRealVar *getCoefficient(const RooArgList &term)
+   {
+      return static_cast<RooRealVar *>(term.at(term.size() - 1));
+   }
+   inline RooRealVar *getExponent(const RooArgList &term, RooRealVar *v)
+   {
+      return static_cast<RooRealVar *>(term.at(_vars.index(v)));
+   }
 
    void addTerm(double coefficient);
    void addTerm(double coefficient, const RooAbsCollection &exponents);
    void addTerm(double coefficient, const RooAbsReal &var1, int exp1);
    void addTerm(double coefficient, const RooAbsReal &var1, int exp1, const RooAbsReal &var2, int exp2);
 
    static std::unique_ptr<RooPolyFunc>
-   taylorExpand(const char *name, const char *title, RooAbsReal &func, const RooAbsCollection &observables,
-                std::vector<double> const &observableValues, int order = 1, double eps1 = 1e-6, double eps2 = 1e-3);
-   static std::unique_ptr<RooPolyFunc> taylorExpand(const char *name, const char *title, RooAbsReal &func,
-                                                    const RooAbsCollection &observables, double observablesValue = 0.0,
-                                                    int order = 1, double eps = 1e-6, double eps2 = 1e-3);
+   taylorExpand(const char *name, const char *title, RooAbsReal &func, const RooArgList &observables, int order = 1,
+                std::vector<double> const &observableValues = {}, double eps1 = 1e-6, double eps2 = 1e-3);
 
 protected:
    void setCoordinate(const RooAbsCollection &observables, std::vector<double> const &observableValues);

roofit/roofitcore/inc/RooSimSplitGenContext.h

@@ -48,7 +48,7 @@ class RooSimSplitGenContext : public RooAbsGenContext {
   RooSimSplitGenContext(const RooSimSplitGenContext& other) ;
 
   RooAbsCategoryLValue* _idxCat ;           ///< Clone of index category
-  RooArgSet*            _idxCatSet ;        ///< Owner of index category components
+  RooArgSet             _idxCatSet ;        ///< Owner of index category components
   const RooSimultaneous *_pdf ;             ///< Original PDF
   std::vector<RooAbsGenContext*> _gcList ;  ///< List of component generator contexts
   std::vector<int>               _gcIndex ; ///< Index value corresponding to component

roofit/roofitcore/src/RooAICRegistry.cxx

@@ -34,7 +34,19 @@ associated normalization and integration sets.
 using namespace std;
 
 ClassImp(RooAICRegistry);
-;
+
+namespace {
+
+RooArgSet * makeSnapshot(RooArgSet const* set) {
+  if(!set) {
+    return nullptr;
+  }
+  auto out = new RooArgSet;
+  set->snapshot(*out, false);
+  return out;
+}
+
+}
 
 ////////////////////////////////////////////////////////////////////////////////
 
@@ -63,10 +75,10 @@ RooAICRegistry::RooAICRegistry(const RooAICRegistry& other)
     _asArr3.resize(size, 0);
     _asArr4.resize(size, 0);
     for(UInt_t i = 0; i < size; ++i) {
-      _asArr1[i] = other._asArr1[i] ? ((RooArgSet*)other._asArr1[i]->snapshot(false)) : 0;
-      _asArr2[i] = other._asArr2[i] ? ((RooArgSet*)other._asArr2[i]->snapshot(false)) : 0;
-      _asArr3[i] = other._asArr3[i] ? ((RooArgSet*)other._asArr3[i]->snapshot(false)) : 0;
-      _asArr4[i] = other._asArr4[i] ? ((RooArgSet*)other._asArr4[i]->snapshot(false)) : 0;
+      _asArr1[i] = makeSnapshot(other._asArr1[i]);
+      _asArr2[i] = makeSnapshot(other._asArr2[i]);
+      _asArr3[i] = makeSnapshot(other._asArr3[i]);
+      _asArr4[i] = makeSnapshot(other._asArr4[i]);
     }
   }
 }
@@ -133,10 +145,10 @@ Int_t RooAICRegistry::store(const std::vector<Int_t>& codeList, RooArgSet* set1,
 
   // Store code list and return index
   _clArr.push_back(codeList);
-  _asArr1.push_back(set1 ? (RooArgSet*)set1->snapshot(false) : 0);
-  _asArr2.push_back(set2 ? (RooArgSet*)set2->snapshot(false) : 0);
-  _asArr3.push_back(set3 ? (RooArgSet*)set3->snapshot(false) : 0);
-  _asArr4.push_back(set4 ? (RooArgSet*)set4->snapshot(false) : 0);
+  _asArr1.emplace_back(makeSnapshot(set1));
+  _asArr2.emplace_back(makeSnapshot(set2));
+  _asArr3.emplace_back(makeSnapshot(set3));
+  _asArr4.emplace_back(makeSnapshot(set4));
 
   if (set1) delete set1 ;
   if (set2) delete set2 ;

roofit/roofitcore/src/RooAbsData.cxx

@@ -2165,8 +2165,8 @@ Roo1DTable* RooAbsData::table(const RooAbsCategory& cat, const char* cuts, const
     }
 
     // Clone derived variable
-    tableSet.reset(static_cast<RooArgSet*>(RooArgSet(cat).snapshot(true)));
-    if (!tableSet) {
+    tableSet = std::make_unique<RooArgSet>();
+    if (RooArgSet(cat).snapshot(*tableSet, true)) {
       coutE(Plotting) << "RooTreeData::table(" << GetName() << ") Couldn't deep-clone table category, abort." << std::endl;
       return nullptr;
     }

roofit/roofitcore/src/RooAbsMinimizerFcn.cxx

@@ -69,17 +69,21 @@ RooAbsMinimizerFcn::RooAbsMinimizerFcn(RooArgList paramList, RooMinimizer *conte
    _nDim = _floatParamList->getSize();
 
    // Save snapshot of initial lists
-   _initFloatParamList.reset((RooArgList *)_floatParamList->snapshot(false));
-   _initConstParamList.reset((RooArgList *)_constParamList->snapshot(false));
+   _initFloatParamList = std::make_unique<RooArgList>();
+   _initConstParamList = std::make_unique<RooArgList>();
+   _floatParamList->snapshot(*_initFloatParamList, false);
+   _constParamList->snapshot(*_initConstParamList, false);
 }
 
 RooAbsMinimizerFcn::RooAbsMinimizerFcn(const RooAbsMinimizerFcn &other)
    : _context(other._context), _maxFCN(other._maxFCN), _funcOffset(other._funcOffset), _numBadNLL(other._numBadNLL),
      _evalCounter(other._evalCounter), _nDim(other._nDim), _optConst(other._optConst),
      _floatParamList(new RooArgList(*other._floatParamList)), _constParamList(new RooArgList(*other._constParamList)),
-     _initFloatParamList((RooArgList *)other._initFloatParamList->snapshot(false)),
-     _initConstParamList((RooArgList *)other._initConstParamList->snapshot(false)), _logfile(other._logfile)
+     _initFloatParamList(std::make_unique<RooArgList>()), _initConstParamList(std::make_unique<RooArgList>()),
+     _logfile(other._logfile)
 {
+   other._initFloatParamList->snapshot(*_initFloatParamList, false);
+   other._initConstParamList->snapshot(*_initConstParamList, false);
 }
 
 /// Internal function to synchronize TMinimizer with current
@@ -132,7 +136,7 @@ bool RooAbsMinimizerFcn::synchronizeParameterSettings(std::vector<ROOT::Fit::Par
    }
 
    // Update reference list
-   *_initConstParamList = *_constParamList;
+   _initConstParamList->assign(*_constParamList);
 
    // Synchronize MINUIT with function state
    // Handle floatParamList

roofit/roofitcore/src/RooAbsNumGenerator.cxx

@@ -49,19 +49,18 @@ using namespace std;
 /// cloned and so will not be disturbed during the generation process.
 
 RooAbsNumGenerator::RooAbsNumGenerator(const RooAbsReal &func, const RooArgSet &genVars, bool verbose, const RooAbsReal* maxFuncVal) :
-  _cloneSet(0), _funcClone(0), _funcMaxVal(maxFuncVal), _verbose(verbose), _funcValStore(0), _funcValPtr(0), _cache(0)
+  _funcClone(0), _funcMaxVal(maxFuncVal), _verbose(verbose), _funcValStore(0), _funcValPtr(0), _cache(0)
 {
   // Clone the function and all nodes that it depends on so that this generator
   // is independent of any existing objects.
   RooArgSet nodes(func,func.GetName());
-  _cloneSet= (RooArgSet*) nodes.snapshot(true);
-  if (!_cloneSet) {
+  if (nodes.snapshot(_cloneSet, true)) {
     oocoutE(nullptr, Generation) << "RooAbsNumGenerator::RooAbsNumGenerator(" << func.GetName() << ") Couldn't deep-clone function, abort," << endl ;
     RooErrorHandler::softAbort() ;
   }
 
   // Find the clone in the snapshot list
-  _funcClone = (RooAbsReal*)_cloneSet->find(func.GetName());
+  _funcClone = (RooAbsReal*)_cloneSet.find(func.GetName());
 
 
   // Check that each argument is fundamental, and separate them into
@@ -77,12 +76,12 @@ RooAbsNumGenerator::RooAbsNumGenerator(const RooAbsReal &func, const RooArgSet &
       continue;
     }
     // look for this argument in the generating function's dependents
-    found= (const RooAbsArg*)_cloneSet->find(arg->GetName());
+    found= (const RooAbsArg*)_cloneSet.find(arg->GetName());
     if(found) {
       arg= found;
     } else {
       // clone any variables we generate that we haven't cloned already
-      arg= _cloneSet->addClone(*arg);
+      arg= _cloneSet.addClone(*arg);
     }
     assert(0 != arg);
     // is this argument a category or a real?
@@ -145,7 +144,6 @@ RooAbsNumGenerator::RooAbsNumGenerator(const RooAbsReal &func, const RooArgSet &
 
 RooAbsNumGenerator::~RooAbsNumGenerator()
 {
-  delete _cloneSet;
   delete _cache ;
   delete _funcValStore ;
 }

roofit/roofitcore/src/RooAbsNumGenerator.h

@@ -27,7 +27,7 @@ class RooNumGenConfig ;
 
 class RooAbsNumGenerator {
 public:
-  RooAbsNumGenerator() : _cloneSet(nullptr), _funcClone(nullptr), _funcMaxVal(nullptr), _verbose(false), _isValid(false), _funcValStore(nullptr), _funcValPtr(nullptr), _cache(nullptr) {}
+  RooAbsNumGenerator() : _funcClone(nullptr), _funcMaxVal(nullptr), _verbose(false), _isValid(false), _funcValStore(nullptr), _funcValPtr(nullptr), _cache(nullptr) {}
   RooAbsNumGenerator(const RooAbsReal &func, const RooArgSet &genVars, bool verbose=false, const RooAbsReal* maxFuncVal=nullptr);
   virtual RooAbsNumGenerator* clone(const RooAbsReal&, const RooArgSet& genVars, const RooArgSet& condVars,
                 const RooNumGenConfig& config, bool verbose=false, const RooAbsReal* maxFuncVal=nullptr) const = 0 ;
@@ -61,10 +61,10 @@ class RooAbsNumGenerator {
 
 protected:
 
-  RooArgSet *_cloneSet;                ///< Set owning clone of input function
-  RooAbsReal *_funcClone;              ///< Pointer to top level node of cloned function
-  const RooAbsReal *_funcMaxVal ;      ///< Container for maximum function value
-  RooArgSet _catVars,_realVars ;       ///< Sets of discrete and real valued observabeles
+  RooArgSet _cloneSet;               ///< Set owning clone of input function
+  RooAbsReal *_funcClone;            ///< Pointer to top level node of cloned function
+  const RooAbsReal *_funcMaxVal ;    ///< Container for maximum function value
+  RooArgSet _catVars,_realVars ;     ///< Sets of discrete and real valued observabeles
   bool _verbose, _isValid;           ///< Verbose and valid flag
   RooRealVar *_funcValStore,*_funcValPtr; ///< RRVs storing function value in context and in output dataset
 

roofit/roofitcore/src/RooAbsOptTestStatistic.cxx

@@ -161,7 +161,11 @@ RooAbsOptTestStatistic::RooAbsOptTestStatistic(const RooAbsOptTestStatistic& oth
     _funcObsSet = 0 ;
     _funcCloneSet = 0 ;
     _funcClone = 0 ;
-    _normSet = other._normSet ? ((RooArgSet*) other._normSet->snapshot()) : 0 ;
+    _normSet = nullptr;
+    if(other._normSet) {
+      _normSet = new RooArgSet;
+      other._normSet->snapshot(*_normSet);
+    }
     _projDeps = 0 ;
     _origFunc = 0 ;
     _origData = 0 ;
@@ -221,7 +225,8 @@ void RooAbsOptTestStatistic::initSlave(RooAbsReal& real, RooAbsData& indata, con
   }
 
   // Store normalization set
-  _normSet = (RooArgSet*) indata.get()->snapshot(false) ;
+  _normSet = new RooArgSet;
+  indata.get()->snapshot(*_normSet, false);
 
   // Expand list of observables with any observables used in parameterized ranges.
   // This NEEDS to be a counting loop since we are inserting during the loop.
@@ -344,7 +349,8 @@ void RooAbsOptTestStatistic::initSlave(RooAbsReal& real, RooAbsData& indata, con
   // Remove projected dependents from normalization set
   if (projDeps.getSize()>0) {
 
-    _projDeps = (RooArgSet*) projDeps.snapshot(false) ;
+    _projDeps = new RooArgSet;
+    projDeps.snapshot(*_projDeps, false) ;
 
     //RooArgSet* tobedel = (RooArgSet*) _normSet->selectCommon(*_projDeps) ;
     _normSet->remove(*_projDeps,true,true) ;

roofit/roofitcore/src/RooAbsReal.cxx

@@ -907,8 +907,8 @@ const RooAbsReal *RooAbsReal::createPlotProjection(const RooArgSet &dependentVar
   if(0 != projectedVars) leafNodes.remove(*projectedVars,true);
 
   // Make a deep-clone of ourself so later operations do not disturb our original state
-  cloneSet= (RooArgSet*)RooArgSet(*this).snapshot(true);
-  if (!cloneSet) {
+  cloneSet = new RooArgSet;
+  if (RooArgSet(*this).snapshot(*cloneSet, true)) {
     coutE(Plotting) << "RooAbsPdf::createPlotProjection(" << GetName() << ") Couldn't deep-clone PDF, abort," << std::endl ;
     return 0 ;
   }
@@ -1176,8 +1176,9 @@ RooDataHist* RooAbsReal::fillDataHist(RooDataHist *hist, const RooArgSet* normSe
 
   // Make deep clone of self and attach to dataset observables
   //RooArgSet* origObs = getObservables(hist) ;
-  std::unique_ptr<RooArgSet> cloneSet{static_cast<RooArgSet*>(RooArgSet(*this).snapshot(true))};
-  RooAbsReal* theClone = (RooAbsReal*) cloneSet->find(GetName()) ;
+  RooArgSet cloneSet;
+  RooArgSet(*this).snapshot(cloneSet, true);
+  RooAbsReal* theClone = static_cast<RooAbsReal*>(cloneSet.find(GetName()));
   theClone->recursiveRedirectServers(*hist->get()) ;
   //const_cast<RooAbsReal*>(this)->recursiveRedirectServers(*hist->get()) ;
 

roofit/roofitcore/src/RooAbsSelfCachedPdf.cxx

@@ -78,8 +78,9 @@ void RooAbsSelfCachedPdf::fillCacheObject(RooAbsCachedPdf::PdfCacheElem& cache)
   RooDataHist& cacheHist = *cache.hist() ;
 
   // Make deep clone of self in non-caching mde and attach to dataset observables
-  RooArgSet* cloneSet = (RooArgSet*) RooArgSet(*this).snapshot(true) ;
-  RooAbsSelfCachedPdf* clone2 = (RooAbsSelfCachedPdf*) cloneSet->find(GetName()) ;
+  RooArgSet cloneSet;
+  RooArgSet(*this).snapshot(cloneSet, true) ;
+  auto clone2 = static_cast<RooAbsSelfCachedPdf*>(cloneSet.find(GetName()));
   clone2->disableCache(true) ;
   clone2->attachDataSet(cacheHist) ;
 
@@ -91,8 +92,6 @@ void RooAbsSelfCachedPdf::fillCacheObject(RooAbsCachedPdf::PdfCacheElem& cache)
   }
 
   cache.pdf()->setUnitNorm(true) ;
-
-  delete cloneSet ;
 }
 
 

roofit/roofitcore/src/RooAbsSelfCachedReal.cxx

@@ -78,8 +78,9 @@ void RooAbsSelfCachedReal::fillCacheObject(RooAbsCachedReal::FuncCacheElem& cach
   RooDataHist& cacheHist = *cache.hist() ;
 
   // Make deep clone of self in non-caching mde and attach to dataset observables
-  RooArgSet* cloneSet = (RooArgSet*) RooArgSet(*this).snapshot(true) ;
-  RooAbsSelfCachedReal* clone2 = (RooAbsSelfCachedReal*) cloneSet->find(GetName()) ;
+  RooArgSet cloneSet;
+  RooArgSet(*this).snapshot(cloneSet, true) ;
+  RooAbsSelfCachedReal* clone2 = (RooAbsSelfCachedReal*) cloneSet.find(GetName()) ;
   clone2->disableCache(true) ;
   clone2->attachDataSet(cacheHist) ;
 
@@ -89,8 +90,6 @@ void RooAbsSelfCachedReal::fillCacheObject(RooAbsCachedReal::FuncCacheElem& cach
     double wgt = clone2->getVal(obs) ;
     cacheHist.set(i, wgt, 0.);
   }
-
-  delete cloneSet ;
 }
 
 

roofit/roofitcore/src/RooAddGenContext.cxx

@@ -56,7 +56,8 @@ RooAddGenContext::RooAddGenContext(const RooAddPdf &model, const RooArgSet &vars
   ccxcoutI(Generation) << std::endl;
 
   // Constructor. Build an array of generator contexts for each product component PDF
-  _pdfSet.reset(static_cast<RooArgSet*>(RooArgSet(model).snapshot(true)));
+  _pdfSet = std::make_unique<RooArgSet>();
+  RooArgSet(model).snapshot(*_pdfSet, true);
   _pdf = (RooAddPdf*) _pdfSet->find(model.GetName()) ;
   _pdf->setOperMode(RooAbsArg::ADirty,true) ;
 
@@ -70,7 +71,8 @@ RooAddGenContext::RooAddGenContext(const RooAddPdf &model, const RooArgSet &vars
 
   _nComp = model._pdfList.getSize() ;
   _coefThresh.resize(_nComp+1);
-  _vars.reset(static_cast<RooArgSet*>(vars.snapshot(false)));
+  _vars = std::make_unique<RooArgSet>();
+  vars.snapshot(*_vars, false);
 
   for (const auto arg : model._pdfList) {
     auto pdf = dynamic_cast<const RooAbsPdf *>(arg);
@@ -104,12 +106,14 @@ RooAddGenContext::RooAddGenContext(const RooAddModel &model, const RooArgSet &va
   ccxcoutI(Generation) << std::endl;
 
   // Constructor. Build an array of generator contexts for each product component PDF
-  _pdfSet.reset(static_cast<RooArgSet*>(RooArgSet(model).snapshot(true)));
+  _pdfSet = std::make_unique<RooArgSet>();
+  RooArgSet(model).snapshot(*_pdfSet, true);
   _pdf = (RooAbsPdf*) _pdfSet->find(model.GetName()) ;
 
   _nComp = model._pdfList.getSize() ;
   _coefThresh.resize(_nComp+1);
-  _vars.reset(static_cast<RooArgSet*>(vars.snapshot(false)));
+  _vars = std::make_unique<RooArgSet>();
+  vars.snapshot(*_vars, false);
 
   for (const auto obj : model._pdfList) {
     auto pdf = static_cast<RooAbsPdf*>(obj);

roofit/roofitcore/src/RooBinnedGenContext.cxx

@@ -55,8 +55,8 @@ RooBinnedGenContext::RooBinnedGenContext(const RooAbsPdf &model, const RooArgSet
   ccxcoutI(Generation) << endl ;
 
   // Constructor. Build an array of generator contexts for each product component PDF
-  _pdfSet = (RooArgSet*) RooArgSet(model).snapshot(true) ;
-  _pdf = (RooAbsPdf*) _pdfSet->find(model.GetName()) ;
+  RooArgSet(model).snapshot(_pdfSet, true);
+  _pdf = (RooAbsPdf*) _pdfSet.find(model.GetName()) ;
   _pdf->setOperMode(RooAbsArg::ADirty,true) ;
 
   // Fix normalization set of this RooAddPdf
@@ -92,7 +92,6 @@ RooBinnedGenContext::RooBinnedGenContext(const RooAbsPdf &model, const RooArgSet
 RooBinnedGenContext::~RooBinnedGenContext()
 {
   delete _vars ;
-  delete _pdfSet ;
   delete _hist ;
 }