[RF] Set Minuit 2 function directly in MinuitFcnGrad

guitargeek · guitargeek · commit ab9c3d9848ac · 2025-10-03T19:29:48.000+02:00
This means we skip one level of virtual calls in math functors.
diff --git a/math/minuit2/inc/Minuit2/Minuit2Minimizer.h b/math/minuit2/inc/Minuit2/Minuit2Minimizer.h
@@ -279,6 +279,9 @@ class Minuit2Minimizer : public ROOT::Math::Minimizer {
    /// return the minimizer state (containing values, step size , etc..)
    const ROOT::Minuit2::MnUserParameterState &State() { return fState; }
 
+   /// To set the function directly to a Minuit 2 function.
+   void SetFCN(unsigned int nDim, std::unique_ptr<ROOT::Minuit2::FCNBase> fcn);
+
    const ROOT::Minuit2::FCNBase *GetFCN() const { return fMinuitFCN.get(); }
    ROOT::Minuit2::FCNBase *GetFCN() { return fMinuitFCN.get(); }
 
diff --git a/math/minuit2/src/Minuit2Minimizer.cxx b/math/minuit2/src/Minuit2Minimizer.cxx
@@ -1346,6 +1346,12 @@ void Minuit2Minimizer::SetStorageLevel(int level)
       fMinimizer->Builder().SetStorageLevel(level);
 }
 
+void Minuit2Minimizer::SetFCN(unsigned int nDim, std::unique_ptr<ROOT::Minuit2::FCNBase> fcn)
+{
+   fDim = nDim;
+   fMinuitFCN = std::move(fcn);
+}
+
 } // end namespace Minuit2
 
 } // end namespace ROOT
diff --git a/roofit/roofitcore/src/RooAbsMinimizerFcn.h b/roofit/roofitcore/src/RooAbsMinimizerFcn.h
@@ -26,6 +26,8 @@
 #include "RooMinimizer.h"
 #include "RooRealVar.h"
 
+#include <Math/Minimizer.h>
+
 #include <iostream>
 #include <fstream>
 #include <string>
@@ -37,7 +39,7 @@ class RooAbsMinimizerFcn {
    RooAbsMinimizerFcn(RooArgList paramList, RooMinimizer *context);
    virtual ~RooAbsMinimizerFcn() = default;
 
-   virtual bool returnsInMinuit2ParameterSpace() const { return false; }
+   virtual void initMinimizer(ROOT::Math::Minimizer &) = 0;
 
    /// Informs Minuit through its parameter_settings vector of RooFit parameter properties.
    bool synchronizeParameterSettings(std::vector<ROOT::Fit::ParameterSettings> &parameters, bool optConst);
@@ -81,7 +83,6 @@ class RooAbsMinimizerFcn {
 
    /// Enable or disable offsetting on the function to be minimized, which enhances numerical precision.
    virtual void setOffsetting(bool flag) = 0;
-   virtual ROOT::Math::IMultiGenFunction *getMultiGenFcn() = 0;
 
    RooMinimizer::Config const &cfg() const { return _context->_cfg; }
 
diff --git a/roofit/roofitcore/src/RooMinimizer.cxx b/roofit/roofitcore/src/RooMinimizer.cxx
@@ -39,7 +39,7 @@ automatic PDF optimization.
 
 #include "RooMinimizer.h"
 
-#include "RooAbsMinimizerFcn.h"
+#include "RooMinimizerFcn.h"
 #include "RooArgSet.h"
 #include "RooArgList.h"
 #include "RooAbsReal.h"
@@ -67,11 +67,6 @@ automatic PDF optimization.
 #include <TGraph.h>
 #include <TMarker.h>
 
-#ifdef ROOFIT_MULTIPROCESS
-#include <Minuit2/Minuit2Minimizer.h>
-#include <Minuit2/FCNAdapter.h>
-#endif
-
 #include <fstream>
 #include <iostream>
 #include <stdexcept> // logic_error
@@ -905,13 +900,11 @@ std::unique_ptr<RooAbsReal::EvalErrorContext> RooMinimizer::makeEvalErrorContext
 
 bool RooMinimizer::fitFCN()
 {
-   const auto &fcn = *_fcn->getMultiGenFcn();
-
    // fit a user provided FCN function
    // create fit parameter settings
 
    // Check number of parameters
-   unsigned int npar = fcn.NDim();
+   unsigned int npar = getNPar();
    if (npar == 0) {
       coutE(Minimization) << "RooMinimizer::fitFCN(): FCN function has zero parameters" << std::endl;
       return false;
@@ -1135,19 +1128,7 @@ bool RooMinimizer::calculateMinosErrors()
 void RooMinimizer::initMinimizer()
 {
    _minimizer = std::unique_ptr<ROOT::Math::Minimizer>(_config.CreateMinimizer());
-   _minimizer->SetFunction(*_fcn->getMultiGenFcn());
-#ifdef ROOFIT_MULTIPROCESS
-   if (_fcn->returnsInMinuit2ParameterSpace()) {
-      auto &rooFcn = dynamic_cast<RooFit::TestStatistics::MinuitFcnGrad &>(*_fcn);
-      auto &minim = dynamic_cast<ROOT::Minuit2::Minuit2Minimizer &>(*_minimizer);
-      auto &adapter = dynamic_cast<ROOT::Minuit2::FCNAdapter &>(*minim.GetFCN());
-      adapter.SetGradWithPrevResultFunction([&rooFcn](const double *params, double *grad, double *previous_grad,
-                                                      double *previous_g2, double *previous_gstep) {
-         rooFcn.GradientWithPrevResult(params, grad, previous_grad, previous_g2, previous_gstep);
-      });
-      adapter.SetReturnsInMinuit2ParameterSpace(true);
-   }
-#endif
+   _fcn->initMinimizer(*_minimizer);
    _minimizer->SetVariables(_config.ParamsSettings().begin(), _config.ParamsSettings().end());
 
    if (_cfg.setInitialCovariance) {
diff --git a/roofit/roofitcore/src/RooMinimizerFcn.cxx b/roofit/roofitcore/src/RooMinimizerFcn.cxx
@@ -179,4 +179,9 @@ RooArgSet RooMinimizerFcn::freezeDisconnectedParameters() const
    return changedSet;
 }
 
+void RooMinimizerFcn::initMinimizer(ROOT::Math::Minimizer &minim)
+{
+   minim.SetFunction(*_multiGenFcn);
+}
+
 /// \endcond
diff --git a/roofit/roofitcore/src/RooMinimizerFcn.h b/roofit/roofitcore/src/RooMinimizerFcn.h
@@ -35,13 +35,14 @@ class RooMinimizerFcn : public RooAbsMinimizerFcn {
 public:
    RooMinimizerFcn(RooAbsReal *funct, RooMinimizer *context);
 
+   void initMinimizer(ROOT::Math::Minimizer &) override;
+
    std::string getFunctionName() const override;
    std::string getFunctionTitle() const override;
 
    void setOptimizeConstOnFunction(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) override;
 
    void setOffsetting(bool flag) override;
-   ROOT::Math::IMultiGenFunction *getMultiGenFcn() override { return _multiGenFcn.get(); }
 
    double operator()(const double *x) const;
    void evaluateGradient(const double *x, double *out) const;
diff --git a/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.cxx b/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.cxx
@@ -17,37 +17,14 @@
 #include "RooAbsPdf.h"
 #include "RooNaNPacker.h"
 
+#include <Minuit2/Minuit2Minimizer.h>
+#include <Minuit2/FCNAdapter.h>
+
 #include <iomanip> // std::setprecision
 
 namespace RooFit {
 namespace TestStatistics {
 
-namespace {
-
-class MinuitGradFunctor : public ROOT::Math::IMultiGradFunction {
-
-public:
-   MinuitGradFunctor(MinuitFcnGrad const &fcn) : _fcn{fcn} {}
-
-   ROOT::Math::IMultiGradFunction *Clone() const override { return new MinuitGradFunctor(_fcn); }
-
-   unsigned int NDim() const override { return _fcn.getNDim(); }
-
-   void Gradient(const double *x, double *grad) const override { return _fcn.Gradient(x, grad); }
-
-private:
-   double DoEval(const double *x) const override { return _fcn(x); }
-
-   double DoDerivative(double const * /*x*/, unsigned int /*icoord*/) const override
-   {
-      throw std::runtime_error("MinuitGradFunctor::DoDerivative is not implemented, please use Gradient instead.");
-   }
-
-   MinuitFcnGrad const &_fcn;
-};
-
-} // namespace
-
 /** \class MinuitFcnGrad
  *
  * \brief Minuit-RooMinimizer interface which synchronizes parameter data and coordinates evaluation of likelihood
@@ -75,10 +52,7 @@ class MinuitGradFunctor : public ROOT::Math::IMultiGradFunction {
 MinuitFcnGrad::MinuitFcnGrad(const std::shared_ptr<RooFit::TestStatistics::RooAbsL> &absL, RooMinimizer *context,
                              std::vector<ROOT::Fit::ParameterSettings> &parameters, LikelihoodMode likelihoodMode,
                              LikelihoodGradientMode likelihoodGradientMode)
-   : RooAbsMinimizerFcn(*absL->getParameters(), context),
-     _minuitInternalX(getNDim(), 0),
-     _minuitExternalX(getNDim(), 0),
-     _multiGenFcn{std::make_unique<MinuitGradFunctor>(*this)}
+   : RooAbsMinimizerFcn(*absL->getParameters(), context), _minuitInternalX(getNDim(), 0), _minuitExternalX(getNDim(), 0)
 {
    synchronizeParameterSettings(parameters, true);
 
@@ -266,5 +240,22 @@ bool MinuitFcnGrad::Synchronize(std::vector<ROOT::Fit::ParameterSettings> &param
    return returnee;
 }
 
+void MinuitFcnGrad::initMinimizer(ROOT::Math::Minimizer &minim)
+{
+   auto &rooFcn = *this;
+   auto adapter = std::make_unique<ROOT::Minuit2::FCNAdapter>(
+      [&rooFcn](double const *params) { return rooFcn(params); }, minim.ErrorDef());
+   adapter->SetGradientFunction(
+      [&rooFcn](double const *params, double *grad) { return rooFcn.Gradient(params, grad); });
+   adapter->SetGradWithPrevResultFunction([&rooFcn](const double *params, double *grad, double *previous_grad,
+                                                    double *previous_g2, double *previous_gstep) {
+      rooFcn.GradientWithPrevResult(params, grad, previous_grad, previous_g2, previous_gstep);
+   });
+   adapter->SetReturnsInMinuit2ParameterSpace(rooFcn.returnsInMinuit2ParameterSpace());
+
+   auto &minuit = dynamic_cast<ROOT::Minuit2::Minuit2Minimizer &>(minim);
+   minuit.SetFCN(getNDim(), std::move(adapter));
+}
+
 } // namespace TestStatistics
 } // namespace RooFit
diff --git a/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.h b/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.h
@@ -34,18 +34,18 @@ class MinuitFcnGrad : public RooAbsMinimizerFcn {
                  std::vector<ROOT::Fit::ParameterSettings> &parameters, LikelihoodMode likelihoodMode,
                  LikelihoodGradientMode likelihoodGradientMode);
 
+   void initMinimizer(ROOT::Math::Minimizer &) override;
+
    /// Overridden from RooAbsMinimizerFcn to include gradient strategy synchronization.
    bool Synchronize(std::vector<ROOT::Fit::ParameterSettings> &parameter_settings) override;
 
-   bool returnsInMinuit2ParameterSpace() const override { return _gradient->usesMinuitInternalValues(); }
+   bool returnsInMinuit2ParameterSpace() const { return _gradient->usesMinuitInternalValues(); }
 
    inline void setOptimizeConstOnFunction(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) override
    {
       applyToLikelihood([&](auto &l) { l.constOptimizeTestStatistic(opcode, doAlsoTrackingOpt); });
    }
 
-   ROOT::Math::IMultiGenFunction *getMultiGenFcn() override { return _multiGenFcn.get(); }
-
    double operator()(const double *x) const;
 
    /// IMultiGradFunction overrides necessary for Minuit
@@ -93,8 +93,6 @@ class MinuitFcnGrad : public RooAbsMinimizerFcn {
    mutable bool offsets_reset_ = false;
    void syncOffsets() const;
 
-   std::unique_ptr<ROOT::Math::IMultiGradFunction> _multiGenFcn;
-
    mutable bool _minuitInternalRooFitXMismatch = false;
 };
 

Original file line number	Diff line number	Diff line change
`@@ -179,4 +179,9 @@ RooArgSet RooMinimizerFcn::freezeDisconnectedParameters() const`
`179`	`179`	`return changedSet;`
`180`	`180`	`}`
`181`	`181`
	`182`	`+void RooMinimizerFcn::initMinimizer(ROOT::Math::Minimizer &minim)`
	`183`	`+{`
	`184`	`+ minim.SetFunction(*_multiGenFcn);`
	`185`	`+}`
	`186`	`+`
`182`	`187`	`/// \endcond`