Skip to content

Commit 8a03ed8

Browse files
guitargeekguitargeek
committed
[RF] Set Minuit 2 function directly in MinuitFcnGrad
This means we skip one level of virtual calls in math functors.
1 parent 880efa4 commit 8a03ed8

File tree

11 files changed

+54
-66
lines changed

11 files changed

+54
-66
lines changed

‎math/minuit2/inc/Minuit2/Minuit2Minimizer.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,9 @@ class Minuit2Minimizer : public ROOT::Math::Minimizer {
279279
/// return the minimizer state (containing values, step size , etc..)
280280
const ROOT::Minuit2::MnUserParameterState &State() { return fState; }
281281

282+
/// To set the function directly to a Minuit 2 function.
283+
void SetFCN(unsigned int nDim, std::unique_ptr<ROOT::Minuit2::FCNBase> fcn);
284+
282285
const ROOT::Minuit2::FCNBase *GetFCN() const { return fMinuitFCN.get(); }
283286
ROOT::Minuit2::FCNBase *GetFCN() { return fMinuitFCN.get(); }
284287

‎math/minuit2/src/Minuit2Minimizer.cxx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,6 +1346,12 @@ void Minuit2Minimizer::SetStorageLevel(int level)
13461346
fMinimizer->Builder().SetStorageLevel(level);
13471347
}
13481348

1349+
void Minuit2Minimizer::SetFCN(unsigned int nDim, std::unique_ptr<ROOT::Minuit2::FCNBase> fcn)
1350+
{
1351+
fDim = nDim;
1352+
fMinuitFCN = std::move(fcn);
1353+
}
1354+
13491355
} // end namespace Minuit2
13501356

13511357
} // end namespace ROOT

‎roofit/roofitcore/CMakeLists.txt‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010
############################################################################
1111

1212
if(roofit_multiprocess)
13-
set(RooFitMPTestStatisticsSources src/TestStatistics/LikelihoodJob.cxx src/TestStatistics/LikelihoodGradientJob.cxx)
13+
set(RooFitMPTestStatisticsSources
14+
src/TestStatistics/LikelihoodGradientJob.cxx
15+
src/TestStatistics/LikelihoodJob.cxx
16+
src/TestStatistics/MinuitFcnGrad.cxx
17+
)
1418
list(APPEND EXTRA_LIBRARIES RooFitMultiProcess)
1519
#FIXME: The ProcessTimer.h exposes json in its interface:
1620
list(APPEND EXTRA_LIBRARIES nlohmann_json::nlohmann_json)
@@ -436,7 +440,6 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooFitCore
436440
src/TestStatistics/LikelihoodGradientWrapper.cxx
437441
src/TestStatistics/LikelihoodSerial.cxx
438442
src/TestStatistics/LikelihoodWrapper.cxx
439-
src/TestStatistics/MinuitFcnGrad.cxx
440443
src/TestStatistics/RooAbsL.cxx
441444
src/TestStatistics/RooBinnedL.cxx
442445
src/TestStatistics/RooRealL.cxx

‎roofit/roofitcore/inc/RooMinimizer.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,21 @@
1717
#ifndef ROO_MINIMIZER
1818
#define ROO_MINIMIZER
1919

20-
#include <RooFit/TestStatistics/RooAbsL.h>
21-
#include <RooFit/TestStatistics/LikelihoodWrapper.h>
22-
#include <RooFit/TestStatistics/LikelihoodGradientWrapper.h>
20+
#include <RooAbsReal.h>
2321

2422
#include <TStopwatch.h>
2523
#include <TMatrixDSymfwd.h>
2624

2725
#include <Fit/FitConfig.h>
2826

2927
#include <fstream>
28+
#include <map>
3029
#include <memory>
3130
#include <string>
3231
#include <utility>
3332
#include <vector>
3433

3534
class RooAbsMinimizerFcn;
36-
class RooAbsReal;
3735
class RooFitResult;
3836
class RooArgList;
3937
class RooRealVar;

‎roofit/roofitcore/src/RooAbsMinimizerFcn.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
#include "RooMinimizer.h"
2727
#include "RooRealVar.h"
2828

29+
#include <Math/Minimizer.h>
30+
2931
#include <iostream>
3032
#include <fstream>
3133
#include <string>
@@ -37,7 +39,7 @@ class RooAbsMinimizerFcn {
3739
RooAbsMinimizerFcn(RooArgList paramList, RooMinimizer *context);
3840
virtual ~RooAbsMinimizerFcn() = default;
3941

40-
virtual bool returnsInMinuit2ParameterSpace() const { return false; }
42+
virtual void initMinimizer(ROOT::Math::Minimizer &) = 0;
4143

4244
/// Informs Minuit through its parameter_settings vector of RooFit parameter properties.
4345
bool synchronizeParameterSettings(std::vector<ROOT::Fit::ParameterSettings> &parameters, bool optConst);
@@ -81,7 +83,6 @@ class RooAbsMinimizerFcn {
8183

8284
/// Enable or disable offsetting on the function to be minimized, which enhances numerical precision.
8385
virtual void setOffsetting(bool flag) = 0;
84-
virtual ROOT::Math::IMultiGenFunction *getMultiGenFcn() = 0;
8586

8687
RooMinimizer::Config const &cfg() const { return _context->_cfg; }
8788

‎roofit/roofitcore/src/RooMinimizer.cxx‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ automatic PDF optimization.
5353
#include "RooHelpers.h"
5454
#include "RooMinimizerFcn.h"
5555
#include "RooFitResult.h"
56-
#include "TestStatistics/MinuitFcnGrad.h"
5756
#include "RooFit/TestStatistics/RooAbsL.h"
5857
#include "RooFit/TestStatistics/RooRealL.h"
5958
#ifdef ROOFIT_MULTIPROCESS
59+
#include "TestStatistics/MinuitFcnGrad.h"
6060
#include "RooFit/MultiProcess/Config.h"
6161
#include "RooFit/MultiProcess/ProcessTimer.h"
6262
#endif
@@ -67,11 +67,6 @@ automatic PDF optimization.
6767
#include <TGraph.h>
6868
#include <TMarker.h>
6969

70-
#ifdef ROOFIT_MULTIPROCESS
71-
#include <Minuit2/Minuit2Minimizer.h>
72-
#include <Minuit2/FCNAdapter.h>
73-
#endif
74-
7570
#include <fstream>
7671
#include <iostream>
7772
#include <stdexcept> // logic_error
@@ -905,13 +900,11 @@ std::unique_ptr<RooAbsReal::EvalErrorContext> RooMinimizer::makeEvalErrorContext
905900

906901
bool RooMinimizer::fitFCN()
907902
{
908-
const auto &fcn = *_fcn->getMultiGenFcn();
909-
910903
// fit a user provided FCN function
911904
// create fit parameter settings
912905

913906
// Check number of parameters
914-
unsigned int npar = fcn.NDim();
907+
unsigned int npar = getNPar();
915908
if (npar == 0) {
916909
coutE(Minimization) << "RooMinimizer::fitFCN(): FCN function has zero parameters" << std::endl;
917910
return false;
@@ -1135,19 +1128,7 @@ bool RooMinimizer::calculateMinosErrors()
11351128
void RooMinimizer::initMinimizer()
11361129
{
11371130
_minimizer = std::unique_ptr<ROOT::Math::Minimizer>(_config.CreateMinimizer());
1138-
_minimizer->SetFunction(*_fcn->getMultiGenFcn());
1139-
#ifdef ROOFIT_MULTIPROCESS
1140-
if (_fcn->returnsInMinuit2ParameterSpace()) {
1141-
auto &rooFcn = dynamic_cast<RooFit::TestStatistics::MinuitFcnGrad &>(*_fcn);
1142-
auto &minim = dynamic_cast<ROOT::Minuit2::Minuit2Minimizer &>(*_minimizer);
1143-
auto &adapter = dynamic_cast<ROOT::Minuit2::FCNAdapter &>(*minim.GetFCN());
1144-
adapter.SetGradWithPrevResultFunction([&rooFcn](const double *params, double *grad, double *previous_grad,
1145-
double *previous_g2, double *previous_gstep) {
1146-
rooFcn.GradientWithPrevResult(params, grad, previous_grad, previous_g2, previous_gstep);
1147-
});
1148-
adapter.SetReturnsInMinuit2ParameterSpace(true);
1149-
}
1150-
#endif
1131+
_fcn->initMinimizer(*_minimizer);
11511132
_minimizer->SetVariables(_config.ParamsSettings().begin(), _config.ParamsSettings().end());
11521133

11531134
if (_cfg.setInitialCovariance) {

‎roofit/roofitcore/src/RooMinimizerFcn.cxx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,4 +179,9 @@ RooArgSet RooMinimizerFcn::freezeDisconnectedParameters() const
179179
return changedSet;
180180
}
181181

182+
void RooMinimizerFcn::initMinimizer(ROOT::Math::Minimizer &minim)
183+
{
184+
minim.SetFunction(*_multiGenFcn);
185+
}
186+
182187
/// \endcond

‎roofit/roofitcore/src/RooMinimizerFcn.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,14 @@ class RooMinimizerFcn : public RooAbsMinimizerFcn {
3535
public:
3636
RooMinimizerFcn(RooAbsReal *funct, RooMinimizer *context);
3737

38+
void initMinimizer(ROOT::Math::Minimizer &) override;
39+
3840
std::string getFunctionName() const override;
3941
std::string getFunctionTitle() const override;
4042

4143
void setOptimizeConstOnFunction(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) override;
4244

4345
void setOffsetting(bool flag) override;
44-
ROOT::Math::IMultiGenFunction *getMultiGenFcn() override { return _multiGenFcn.get(); }
4546

4647
double operator()(const double *x) const;
4748
void evaluateGradient(const double *x, double *out) const;

‎roofit/roofitcore/src/TestStatistics/LikelihoodGradientJob.cxx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include "LikelihoodGradientJob.h"
1414

15+
#include <RooFit/TestStatistics/LikelihoodWrapper.h>
1516
#include "RooFit/MultiProcess/JobManager.h"
1617
#include "RooFit/MultiProcess/Messenger.h"
1718
#include "RooFit/MultiProcess/ProcessTimer.h"

‎roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.cxx‎

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,37 +17,14 @@
1717
#include "RooAbsPdf.h"
1818
#include "RooNaNPacker.h"
1919

20+
#include <Minuit2/Minuit2Minimizer.h>
21+
#include <Minuit2/FCNAdapter.h>
22+
2023
#include <iomanip> // std::setprecision
2124

2225
namespace RooFit {
2326
namespace TestStatistics {
2427

25-
namespace {
26-
27-
class MinuitGradFunctor : public ROOT::Math::IMultiGradFunction {
28-
29-
public:
30-
MinuitGradFunctor(MinuitFcnGrad const &fcn) : _fcn{fcn} {}
31-
32-
ROOT::Math::IMultiGradFunction *Clone() const override { return new MinuitGradFunctor(_fcn); }
33-
34-
unsigned int NDim() const override { return _fcn.getNDim(); }
35-
36-
void Gradient(const double *x, double *grad) const override { return _fcn.Gradient(x, grad); }
37-
38-
private:
39-
double DoEval(const double *x) const override { return _fcn(x); }
40-
41-
double DoDerivative(double const * /*x*/, unsigned int /*icoord*/) const override
42-
{
43-
throw std::runtime_error("MinuitGradFunctor::DoDerivative is not implemented, please use Gradient instead.");
44-
}
45-
46-
MinuitFcnGrad const &_fcn;
47-
};
48-
49-
} // namespace
50-
5128
/** \class MinuitFcnGrad
5229
*
5330
* \brief Minuit-RooMinimizer interface which synchronizes parameter data and coordinates evaluation of likelihood
@@ -75,10 +52,7 @@ class MinuitGradFunctor : public ROOT::Math::IMultiGradFunction {
7552
MinuitFcnGrad::MinuitFcnGrad(const std::shared_ptr<RooFit::TestStatistics::RooAbsL> &absL, RooMinimizer *context,
7653
std::vector<ROOT::Fit::ParameterSettings> &parameters, LikelihoodMode likelihoodMode,
7754
LikelihoodGradientMode likelihoodGradientMode)
78-
: RooAbsMinimizerFcn(*absL->getParameters(), context),
79-
_minuitInternalX(getNDim(), 0),
80-
_minuitExternalX(getNDim(), 0),
81-
_multiGenFcn{std::make_unique<MinuitGradFunctor>(*this)}
55+
: RooAbsMinimizerFcn(*absL->getParameters(), context), _minuitInternalX(getNDim(), 0), _minuitExternalX(getNDim(), 0)
8256
{
8357
synchronizeParameterSettings(parameters, true);
8458

@@ -266,5 +240,22 @@ bool MinuitFcnGrad::Synchronize(std::vector<ROOT::Fit::ParameterSettings> &param
266240
return returnee;
267241
}
268242

243+
void MinuitFcnGrad::initMinimizer(ROOT::Math::Minimizer &minim)
244+
{
245+
auto &rooFcn = *this;
246+
auto adapter = std::make_unique<ROOT::Minuit2::FCNAdapter>(
247+
[&rooFcn](double const *params) { return rooFcn(params); }, minim.ErrorDef());
248+
adapter->SetGradientFunction(
249+
[&rooFcn](double const *params, double *grad) { return rooFcn.Gradient(params, grad); });
250+
adapter->SetGradWithPrevResultFunction([&rooFcn](const double *params, double *grad, double *previous_grad,
251+
double *previous_g2, double *previous_gstep) {
252+
rooFcn.GradientWithPrevResult(params, grad, previous_grad, previous_g2, previous_gstep);
253+
});
254+
adapter->SetReturnsInMinuit2ParameterSpace(rooFcn.returnsInMinuit2ParameterSpace());
255+
256+
auto &minuit = dynamic_cast<ROOT::Minuit2::Minuit2Minimizer &>(minim);
257+
minuit.SetFCN(getNDim(), std::move(adapter));
258+
}
259+
269260
} // namespace TestStatistics
270261
} // namespace RooFit

0 commit comments

Comments
 (0)