[RF] Update xroofit and add a first test for it

roofit/xroofit/CMakeLists.txt

@@ -33,3 +33,5 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooFitXRooFit
         )
 
 target_include_directories(RooFitXRooFit PRIVATE inc/RooFit)
+
+ROOT_ADD_TEST_SUBDIRECTORY(test)

roofit/xroofit/inc/RooFit/xRooFit/xRooFit.h

@@ -66,6 +66,8 @@ class xRooFit {
    static RooCmdArg ReuseNLL(bool flag); // if should try to reuse the NLL object when it changes dataset
    static RooCmdArg Tolerance(double value);
    static RooCmdArg StrategySequence(const char *stratSeq); // control minimization strategy sequence
+   static RooCmdArg MaxIterations(int nIterations);
+
    static constexpr double OBS = std::numeric_limits<double>::quiet_NaN();
 
    // Helper function for matching precision of a value and its error

roofit/xroofit/inc/RooFit/xRooFit/xRooNLLVar.h

@@ -38,7 +38,6 @@ class RooAbsReal;
 class RooAbsPdf;
 class RooAbsData;
 class RooAbsCollection;
-class RooNLLVar;
 class RooConstraintSum;
 class RooRealVar;
 class RooCmdArg;
@@ -93,6 +92,7 @@ class xRooNLLVar : public std::shared_ptr<RooAbsReal> {
 
    class xRooFitResult : public std::shared_ptr<const RooFitResult> {
    public:
+      xRooFitResult(const RooFitResult &fr);
       xRooFitResult(const std::shared_ptr<xRooNode> &in,
                     const std::shared_ptr<xRooNLLVar> &nll = nullptr); // : fNode(in) { }
       const RooFitResult *operator->() const;
@@ -430,19 +430,27 @@ class xRooNLLVar : public std::shared_ptr<RooAbsReal> {
 
    // total nll should be all these values + constraint term + extended term + simTerm [+binnedDataTerm if activated
    // binnedL option]
-   RooNLLVar *mainTerm() const;
+   /*RooAbsReal *mainTerm() const;*/
    RooConstraintSum *constraintTerm() const;
 
+   double mainTermVal() const;
+   double constraintTermVal() const;
+
    double getEntryVal(size_t entry) const; // get the Nll value for a specific entry
-   double extendedTerm() const;
-   double simTerm() const;
-   double binnedDataTerm() const;
+   double extendedTermVal() const;
+   double simTermVal() const;
+   double binnedDataTermVal() const;
    double getEntryBinWidth(size_t entry) const;
 
    double ndof() const;
    double saturatedVal() const;
-   double saturatedConstraintTerm() const;
-   double saturatedMainTerm() const;
+   [[deprecated("Use saturatedConstraintTermVal()")]] double saturatedConstraintTerm() const
+   {
+      return saturatedConstraintTermVal();
+   }
+   double saturatedConstraintTermVal() const;
+   [[deprecated("Use saturatedMainTermVal()")]] double saturatedMainTerm() const { return saturatedMainTermVal(); }
+   double saturatedMainTermVal() const;
    double pgof() const; // a goodness-of-fit pvalue based on profile likelihood of a saturated model
    double mainTermPgof() const;
    double mainTermNdof() const;

roofit/xroofit/inc/RooFit/xRooFit/xRooNode.h

@@ -304,7 +304,7 @@ class xRooNode : public TNamed, public std::vector<std::shared_ptr<xRooNode>> {
    xRooNode components() const; // additive children
    xRooNode factors() const;    // multiplicative children
    xRooNode variations() const; // interpolated children (are bins a form of variation?)
-   xRooNode coefs() const;
+   xRooNode coefs(bool recurse = false) const;
    xRooNode coords(bool setVals = true) const; // will move to the coords in the process if setVals=true
    xRooNode bins() const;
 
@@ -379,17 +379,38 @@ class xRooNode : public TNamed, public std::vector<std::shared_ptr<xRooNode>> {
    bool SetXaxis(int nbins, double low, double high) { return SetXaxis("xaxis", "", nbins, low, high); }
    bool SetXaxis(int nbins, const double *bins) { return SetXaxis("xaxis", "", nbins, bins); }
 
-   std::shared_ptr<TStyle> style(TObject *initObject = nullptr, bool autoCreate = true) const;
+   std::shared_ptr<TStyle>
+   style(TObject *initObject = nullptr, bool autoCreate = true) const; // DEPRECATED: TO BE REMOVED
+   xRooNode styles(TObject *initObject = nullptr, bool autoCreate = true) const;
 
    TAxis *GetXaxis() const;
 
    double GetBinData(int bin, const xRooNode &data = "obsData");
    double GetBinContent(int bin) const { return GetBinContents(bin, bin).at(0); }
    std::vector<double> GetBinContents(int binStart = 1, int binEnd = 0) const; // default will get all bins
-   double GetBinError(int bin, const xRooNode &fr = "") const;
-   std::vector<double> GetBinErrors(int binStart = 1, int binEnd = 0, const xRooNode &fr = "") const;
+   double
+   GetBinError(int bin, const xRooNode &fr = "", int nToys = 0, bool errorsHi = false, bool errorsLo = false) const;
+   std::vector<double> GetBinErrors(int binStart = 1, int binEnd = 0, const xRooNode &fr = "", int nToys = 0,
+                                    bool errorsHi = false, bool errorsLo = false) const;
    std::pair<double, double> IntegralAndError(const xRooNode &fr = "", const char *rangeName = nullptr) const;
 
+   std::vector<double> GetBinErrorsHi(int binStart = 1, int binEnd = 0, const xRooNode &fr = "", int nToys = 0) const
+   {
+      return GetBinErrors(binStart, binEnd, fr, nToys, true, false);
+   }
+   std::vector<double> GetBinErrorsLo(int binStart = 1, int binEnd = 0, const xRooNode &fr = "", int nToys = 0) const
+   {
+      return GetBinErrors(binStart, binEnd, fr, nToys, false, true);
+   }
+   double GetBinErrorHi(int bin, const xRooNode &fr = "", int nToys = 0) const
+   {
+      return GetBinError(bin, fr, nToys, true, false);
+   }
+   double GetBinErrorLo(int bin, const xRooNode &fr = "", int nToys = 0) const
+   {
+      return GetBinError(bin, fr, nToys, false, true);
+   }
+
    // methods to access default content and error
    double GetContent() const { return GetBinContent(fBinNumber); }
    double GetError(const xRooNode &fr = "") const
@@ -401,11 +422,15 @@ class xRooNode : public TNamed, public std::vector<std::shared_ptr<xRooNode>> {
    // methods to access content and covariances of the CHILDREN of a node
    std::vector<double> contents() const;
    TMatrixDSym covariances(const xRooNode &fr = "") const;
-
    xRooNLLVar nll(const xRooNode &_data, std::initializer_list<RooCmdArg> nllOpts) const;
    xRooNLLVar nll(const xRooNode &_data, const RooLinkedList &nllOpts) const;
    xRooNLLVar nll(const xRooNode &_data = "") const; // uses xRooFit::createNLLOption for nllOpts
 
+   xRooNLLVar
+   nll(const char *_data,
+       std::initializer_list<RooCmdArg> nllOpts = {}) const; // exists to have sensible exception reporting in python
+                                                             // (rather than conversion errors, which are incorrect)
+
    xRooNode fitResult(const char *opt = "") const;      // todo: make this 'fitResults'
    void SetFitResult(const RooFitResult *fr = nullptr); // null means will load prefit
    void SetFitResult(const std::shared_ptr<const RooFitResult> &fr) { SetFitResult(fr.get()); }
@@ -443,13 +468,16 @@ class xRooNode : public TNamed, public std::vector<std::shared_ptr<xRooNode>> {
    void SetChecked(bool val = true) { Checked(this, val); }
 
    /** @private */
-   xRooNode histo(const xRooNode &vars = "x", const xRooNode &fr = "", bool content = true, bool errors = true) const;
+   xRooNode histo(const xRooNode &vars = "x", const xRooNode &fr = "", bool content = true, bool errors = true,
+                  bool stack = true, bool errorsHi = false, bool errorsLo = false, int nErrorToys = 0) const;
    /** @private */
    xRooNode filter(const xRooNode &range) const;
 
    TGraph *BuildGraph(RooAbsLValue *v = nullptr, bool includeZeros = false, TVirtualPad *fromPad = nullptr) const;
    TH1 *BuildHistogram(RooAbsLValue *v = nullptr, bool empty = false, bool errors = false, int binStart = 1,
-                       int binEnd = 0, const xRooNode &fr = "") const;
+                       int binEnd = 0, const xRooNode &fr = "", bool errorsHi = false, bool errorsLo = false,
+                       int nErrorToys = 0, TH1 *templateHist = nullptr, bool nostack = true,
+                       bool setInterp = false) const;
    xRooNode mainChild() const;
    void Draw(Option_t *opt = "") override; // *MENU*
 

roofit/xroofit/src/xRooFit.cxx

@@ -52,7 +52,6 @@
 #include "TGraphErrors.h"
 #include "TLegend.h"
 #include "TKey.h"
-#include "../../roofitcore/src/RooAbsTestStatistic.h"
 #include "TPRegexp.h"
 #include "RooStringVar.h"
 
@@ -94,6 +93,11 @@ RooCmdArg xRooFit::StrategySequence(const char *val)
    return RooCmdArg("StrategySequence", 0, 0, 0, 0, val);
 }
 
+RooCmdArg xRooFit::MaxIterations(int val)
+{
+   return RooCmdArg("MaxIterations", val);
+}
+
 xRooNLLVar xRooFit::createNLL(const std::shared_ptr<RooAbsPdf> pdf, const std::shared_ptr<RooAbsData> data,
                               const RooLinkedList &nllOpts)
 {
@@ -628,7 +632,7 @@ class ProgressMonitor : public RooAbsReal {
             // doing a hesse step, estimate progress based on evaluations
             int nRequired = prevPars.size();
             if (nRequired > 1) {
-               nRequired *= (nRequired - 1) / 2;
+               nRequired *= nRequired;
                if (fState == "Hesse3") {
                   nRequired *= 4;
                }
@@ -1133,9 +1137,8 @@ std::shared_ptr<const RooFitResult> xRooFit::minimize(RooAbsReal &nll,
       // }
 
       // only do hesse if was a valid min and not full accurate cov matrix already (can happen if e.g. ran strat2)
-      if (hesse &&
-          (m_strategy(sIdx) == 'h' || ((strategy < 2 || _minimizer.fitter()->GetMinimizer()->CovMatrixStatus() != 3) &&
-                                       _minimizer.fitter()->Result().IsValid()))) {
+      if (hesse && m_hessestrategy.Length() != 0 &&
+          (m_strategy(sIdx) == 'h' || (_minimizer.fitter()->Result().IsValid()))) {
 
          // Note: minima where the covariance was made posdef are deemed 'valid' ...
 
@@ -1171,6 +1174,19 @@ std::shared_ptr<const RooFitResult> xRooFit::minimize(RooAbsReal &nll,
          }
          while (sIdx != -1) {
             hesseStrategy = int(m_hessestrategy(sIdx) - '0');
+
+            if (strategy == 2 && hesseStrategy == 2) {
+               // don't repeat hesse if strategy=2 and hesseStrategy=2, and the matrix was valid
+               if (_minimizer.fitter()->GetMinimizer()->CovMatrixStatus() == 3) {
+                  break;
+               }
+               if (sIdx >= m_hessestrategy.Length() - 1) {
+                  break; // run out of strategies to try, stop
+               }
+               sIdx++;
+               continue;
+            }
+
             _minimizer.fitter()->Config().MinimizerOptions().SetStrategy(hesseStrategy);
             // const_cast<ROOT::Math::IOptions*>(_minimizer.fitter()->Config().MinimizerOptions().ExtraOptions())->SetValue("HessianStepTolerance",0.1);
             // const_cast<ROOT::Math::IOptions*>(_minimizer.fitter()->Config().MinimizerOptions().ExtraOptions())->SetValue("HessianG2Tolerance",0.02);
@@ -1241,6 +1257,7 @@ std::shared_ptr<const RooFitResult> xRooFit::minimize(RooAbsReal &nll,
          if (std::unique_ptr<RooAbsCollection> mpars(floatPars->selectByAttrib("minos", true)); !mpars->empty()) {
             if (auto fff = dynamic_cast<ProgressMonitor *>(_nll); fff) {
                fff->fState = "Minos";
+               fff->counter2 = 0;
             }
             auto _status = _minimizer.minos(*mpars);
             statusHistory.push_back(std::pair("Minos", _status));

roofit/xroofit/src/xRooFitVersion.h

@@ -12,5 +12,5 @@
 
 #pragma once
 
-#define GIT_COMMIT_HASH "32646f9"
-#define GIT_COMMIT_DATE "2024-06-13 17:14:41 +0200"
+#define GIT_COMMIT_HASH "c9465df"
+#define GIT_COMMIT_DATE "2024-11-28 11:49:25 +0000"

roofit/xroofit/src/xRooHypoSpace.cxx

@@ -185,6 +185,17 @@ int xRooNLLVar::xRooHypoSpace::AddPoints(const char *parName, size_t nPoints, do
       throw std::runtime_error("Unknown parameter");
    _par->setAttribute("axis");
 
+   if (low < _par->getMin()) {
+      Warning("AddPoints", "low edge of hypoSpace %g below lower bound of parameter: %g. Changing to lower bound", low,
+              _par->getMin());
+      low = _par->getMin();
+   }
+   if (high > _par->getMax()) {
+      Warning("AddPoints", "high edge of hypoSpace %g above upper bound of parameter: %g. Changing to upper bound",
+              high, _par->getMax());
+      high = _par->getMax();
+   }
+
    if (nPoints == 1) {
       _par->setVal((high + low) * 0.5);
       AddPoint();