Skip to content

Commit 880efa4

Browse files
guitargeekguitargeek
committed
[Minuit2] Use native FCNBase type in NumericalDerivator class
We should avoid leaking the ROOT Math classes into Minuit 2.
1 parent 5cc41e0 commit 880efa4

File tree

5 files changed

+47
-47
lines changed

5 files changed

+47
-47
lines changed

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,10 @@
1818
#ifndef ROOT_Minuit2_NumericalDerivator
1919
#define ROOT_Minuit2_NumericalDerivator
2020

21-
#include <Math/IFunctionfwd.h>
22-
2321
#include "Fit/ParameterSettings.h"
2422
#include "Minuit2/MnParameterTransformation.h"
2523
#include "Minuit2/MnMachinePrecision.h"
24+
#include "Minuit2/FCNBase.h"
2625

2726
#include <ROOT/RSpan.hxx>
2827

@@ -45,19 +44,19 @@ class NumericalDerivator {
4544
NumericalDerivator(double step_tolerance, double grad_tolerance, unsigned int ncycles, double error_level,
4645
bool always_exactly_mimic_minuit2 = true);
4746

48-
void SetupDifferentiate(const ROOT::Math::IBaseFunctionMultiDim *function, const double *cx,
47+
void SetupDifferentiate(unsigned int nDim, const FCNBase *function, const double *cx,
4948
std::span<const ROOT::Fit::ParameterSettings> parameters);
50-
std::vector<DerivatorElement> Differentiate(const ROOT::Math::IBaseFunctionMultiDim *function, const double *x,
49+
std::vector<DerivatorElement> Differentiate(unsigned int nDim, const FCNBase *function, const double *x,
5150
std::span<const ROOT::Fit::ParameterSettings> parameters,
5251
std::span<const DerivatorElement> previous_gradient);
5352

54-
DerivatorElement PartialDerivative(const ROOT::Math::IBaseFunctionMultiDim *function, const double *x,
53+
DerivatorElement PartialDerivative(unsigned int nDim, const FCNBase *function, const double *x,
5554
std::span<const ROOT::Fit::ParameterSettings> parameters,
5655
unsigned int i_component, DerivatorElement previous);
57-
DerivatorElement FastPartialDerivative(const ROOT::Math::IBaseFunctionMultiDim *function,
56+
DerivatorElement FastPartialDerivative(const FCNBase *function,
5857
std::span<const ROOT::Fit::ParameterSettings> parameters,
5958
unsigned int i_component, const DerivatorElement &previous);
60-
DerivatorElement operator()(const ROOT::Math::IBaseFunctionMultiDim *function, const double *x,
59+
DerivatorElement operator()(unsigned int nDim, const FCNBase *function, const double *x,
6160
std::span<const ROOT::Fit::ParameterSettings> parameters, unsigned int i_component,
6261
const DerivatorElement &previous);
6362

@@ -99,7 +98,6 @@ class NumericalDerivator {
9998

10099
unsigned int fNCycles = 2;
101100
bool fAlwaysExactlyMimicMinuit2;
102-
103101
};
104102

105103
std::ostream &operator<<(std::ostream &out, const DerivatorElement &value);

‎math/minuit2/src/NumericalDerivator.cxx‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
#include "Minuit2/NumericalDerivator.h"
2727
#include <cmath>
2828
#include <algorithm>
29-
#include <Math/IFunction.h>
3029
#include <iostream>
3130
#include <TMath.h>
3231
#include <cassert>
@@ -44,51 +43,52 @@ NumericalDerivator::NumericalDerivator(bool always_exactly_mimic_minuit2)
4443

4544
NumericalDerivator::NumericalDerivator(double step_tolerance, double grad_tolerance, unsigned int ncycles,
4645
double error_level, bool always_exactly_mimic_minuit2)
47-
: fStepTolerance(step_tolerance), fGradTolerance(grad_tolerance), fUp(error_level), fNCycles(ncycles),
46+
: fStepTolerance(step_tolerance),
47+
fGradTolerance(grad_tolerance),
48+
fUp(error_level),
49+
fNCycles(ncycles),
4850
fAlwaysExactlyMimicMinuit2(always_exactly_mimic_minuit2)
4951
{
5052
}
5153

52-
NumericalDerivator::NumericalDerivator(const NumericalDerivator &/*other*/) = default;
53-
54+
NumericalDerivator::NumericalDerivator(const NumericalDerivator & /*other*/) = default;
5455

5556
/// This function sets internal state based on input parameters. This state
5657
/// setup is used in the actual (partial) derivative calculations.
57-
void NumericalDerivator::SetupDifferentiate(const ROOT::Math::IBaseFunctionMultiDim *function, const double *cx,
58+
void NumericalDerivator::SetupDifferentiate(unsigned int nDim, const FCNBase *function, const double *cx,
5859
std::span<const ROOT::Fit::ParameterSettings> parameters)
5960
{
6061
assert(function != nullptr && "function is a nullptr");
6162

62-
fVx.resize(function->NDim());
63-
fVxExternal.resize(function->NDim());
64-
fVxFValCache.resize(function->NDim());
65-
std::copy(cx, cx + function->NDim(), fVx.data());
63+
fVx.resize(nDim);
64+
fVxExternal.resize(nDim);
65+
fVxFValCache.resize(nDim);
66+
std::copy(cx, cx + nDim, fVx.data());
6667

6768
// convert to Minuit external parameters
68-
for (unsigned i = 0; i < function->NDim(); i++) {
69+
for (unsigned i = 0; i < nDim; i++) {
6970
fVxExternal[i] = Int2ext(parameters[i], fVx[i]);
7071
}
7172

7273
if (fVx != fVxFValCache) {
7374
fVxFValCache = fVx;
74-
fVal = (*function)(fVxExternal.data()); // value of function at given points
75+
fVal = (*function)(fVxExternal); // value of function at given points
7576
}
7677

7778
fDfmin = 8. * fPrecision.Eps2() * (std::abs(fVal) + fUp);
7879
fVrysml = 8. * fPrecision.Eps() * fPrecision.Eps();
7980
}
8081

81-
DerivatorElement NumericalDerivator::PartialDerivative(const ROOT::Math::IBaseFunctionMultiDim *function,
82-
const double *x,
82+
DerivatorElement NumericalDerivator::PartialDerivative(unsigned int nDim, const FCNBase *function, const double *x,
8383
std::span<const ROOT::Fit::ParameterSettings> parameters,
8484
unsigned int i_component, DerivatorElement previous)
8585
{
86-
SetupDifferentiate(function, x, parameters);
86+
SetupDifferentiate(nDim, function, x, parameters);
8787
return FastPartialDerivative(function, parameters, i_component, previous);
8888
}
8989

9090
// leaves the parameter setup to the caller
91-
DerivatorElement NumericalDerivator::FastPartialDerivative(const ROOT::Math::IBaseFunctionMultiDim *function,
91+
DerivatorElement NumericalDerivator::FastPartialDerivative(const FCNBase *function,
9292
std::span<const ROOT::Fit::ParameterSettings> parameters,
9393
unsigned int i_component, const DerivatorElement &previous)
9494
{
@@ -121,11 +121,11 @@ DerivatorElement NumericalDerivator::FastPartialDerivative(const ROOT::Math::IBa
121121
step_old = step;
122122
fVx[i_component] = xtf + step;
123123
fVxExternal[i_component] = Int2ext(parameters[i_component], fVx[i_component]);
124-
double fs1 = (*function)(fVxExternal.data());
124+
double fs1 = (*function)(fVxExternal);
125125

126126
fVx[i_component] = xtf - step;
127127
fVxExternal[i_component] = Int2ext(parameters[i_component], fVx[i_component]);
128-
double fs2 = (*function)(fVxExternal.data());
128+
double fs2 = (*function)(fVxExternal);
129129

130130
fVx[i_component] = xtf;
131131
fVxExternal[i_component] = Int2ext(parameters[i_component], fVx[i_component]);
@@ -142,24 +142,24 @@ DerivatorElement NumericalDerivator::FastPartialDerivative(const ROOT::Math::IBa
142142
return deriv;
143143
}
144144

145-
DerivatorElement NumericalDerivator::operator()(const ROOT::Math::IBaseFunctionMultiDim *function, const double *x,
145+
DerivatorElement NumericalDerivator::operator()(unsigned int nDim, const FCNBase *function, const double *x,
146146
std::span<const ROOT::Fit::ParameterSettings> parameters,
147147
unsigned int i_component, const DerivatorElement &previous)
148148
{
149-
return PartialDerivative(function, x, parameters, i_component, previous);
149+
return PartialDerivative(nDim, function, x, parameters, i_component, previous);
150150
}
151151

152152
std::vector<DerivatorElement>
153-
NumericalDerivator::Differentiate(const ROOT::Math::IBaseFunctionMultiDim *function, const double *cx,
153+
NumericalDerivator::Differentiate(unsigned int nDim, const FCNBase *function, const double *cx,
154154
std::span<const ROOT::Fit::ParameterSettings> parameters,
155155
std::span<const DerivatorElement> previous_gradient)
156156
{
157-
SetupDifferentiate(function, cx, parameters);
157+
SetupDifferentiate(nDim, function, cx, parameters);
158158

159159
std::vector<DerivatorElement> gradient;
160-
gradient.reserve(function->NDim());
160+
gradient.reserve(nDim);
161161

162-
for (unsigned int ix = 0; ix < function->NDim(); ++ix) {
162+
for (unsigned int ix = 0; ix < nDim; ++ix) {
163163
gradient.emplace_back(FastPartialDerivative(function, parameters, ix, previous_gradient[ix]));
164164
}
165165

‎roofit/roofitcore/inc/RooMinimizer.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,6 @@ class RooMinimizer : public TObject {
188188
/// Return underlying ROOT fitter object
189189
inline auto fitter() { return std::make_unique<FitterInterface>(&_config, _minimizer.get(), _result.get()); }
190190

191-
ROOT::Math::IMultiGenFunction *getMultiGenFcn() const;
192-
193191
int getNPar() const;
194192

195193
void applyCovarianceMatrix(TMatrixDSym const &V);
@@ -218,7 +216,7 @@ class RooMinimizer : public TObject {
218216

219217
int exec(std::string const &algoName, std::string const &statusName);
220218

221-
bool fitFCN(const ROOT::Math::IMultiGenFunction &fcn);
219+
bool fitFCN();
222220

223221
bool calculateHessErrors();
224222
bool calculateMinosErrors();

‎roofit/roofitcore/src/RooMinimizer.cxx‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ int RooMinimizer::minimize(const char *type, const char *alg)
356356
{
357357
auto ctx = makeEvalErrorContext();
358358

359-
bool ret = fitFCN(*_fcn->getMultiGenFcn());
359+
bool ret = fitFCN();
360360
determineStatus(ret);
361361
}
362362
profileStop();
@@ -398,7 +398,7 @@ int RooMinimizer::exec(std::string const &algoName, std::string const &statusNam
398398
ret = calculateMinosErrors();
399399
} else {
400400
_config.SetMinimizer(_cfg.minimizerType.c_str(), algoName.c_str());
401-
ret = fitFCN(*_fcn->getMultiGenFcn());
401+
ret = fitFCN();
402402
}
403403
determineStatus(ret);
404404
}
@@ -780,12 +780,6 @@ void RooMinimizer::profileStop()
780780
}
781781
}
782782

783-
ROOT::Math::IMultiGenFunction *RooMinimizer::getMultiGenFcn() const
784-
{
785-
786-
return _fcn->getMultiGenFcn();
787-
}
788-
789783
////////////////////////////////////////////////////////////////////////////////
790784
/// Apply results of given external covariance matrix. i.e. propagate its errors
791785
/// to all RRV parameter representations and give this matrix instead of the
@@ -909,8 +903,10 @@ std::unique_ptr<RooAbsReal::EvalErrorContext> RooMinimizer::makeEvalErrorContext
909903
return std::make_unique<RooAbsReal::EvalErrorContext>(m);
910904
}
911905

912-
bool RooMinimizer::fitFCN(const ROOT::Math::IMultiGenFunction &fcn)
906+
bool RooMinimizer::fitFCN()
913907
{
908+
const auto &fcn = *_fcn->getMultiGenFcn();
909+
914910
// fit a user provided FCN function
915911
// create fit parameter settings
916912

@@ -1139,7 +1135,7 @@ bool RooMinimizer::calculateMinosErrors()
11391135
void RooMinimizer::initMinimizer()
11401136
{
11411137
_minimizer = std::unique_ptr<ROOT::Math::Minimizer>(_config.CreateMinimizer());
1142-
_minimizer->SetFunction(*getMultiGenFcn());
1138+
_minimizer->SetFunction(*_fcn->getMultiGenFcn());
11431139
#ifdef ROOFIT_MULTIPROCESS
11441140
if (_fcn->returnsInMinuit2ParameterSpace()) {
11451141
auto &rooFcn = dynamic_cast<RooFit::TestStatistics::MinuitFcnGrad &>(*_fcn);

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include "RooMsgService.h"
2121
#include "RooMinimizer.h"
2222

23+
#include "Minuit2/Minuit2Minimizer.h"
24+
#include "Minuit2/FCNAdapter.h"
2325
#include "Minuit2/MnStrategy.h"
2426

2527
namespace RooFit {
@@ -179,9 +181,12 @@ void LikelihoodGradientJob::update_state()
179181
std::copy(offsets_message_begin, offsets_message_end, shared_offset_.offsets().begin());
180182
}
181183

184+
// Since the gradient parallelization only support Minuit 2, we can do this cast
185+
auto &minim = static_cast<ROOT::Minuit2::Minuit2Minimizer &>(*minimizer_->_minimizer);
186+
182187
// note: the next call must stay after the (possible) update of the offset, because it
183188
// calls the likelihood function, so the offset must be correct at this point
184-
gradf_.SetupDifferentiate(minimizer_->getMultiGenFcn(), minuit_internal_x_.data(),
189+
gradf_.SetupDifferentiate(minimizer_->getNPar(), minim.GetFCN(), minuit_internal_x_.data(),
185190
minimizer_->fitter()->Config().ParamsSettings());
186191
}
187192
}
@@ -193,9 +198,12 @@ void LikelihoodGradientJob::update_state()
193198

194199
void LikelihoodGradientJob::run_derivator(unsigned int i_component) const
195200
{
201+
// Since the gradient parallelization only support Minuit 2, we can do this cast
202+
auto &minim = static_cast<ROOT::Minuit2::Minuit2Minimizer &>(*minimizer_->_minimizer);
203+
196204
// Calculate the derivative etc for these parameters
197-
grad_[i_component] = gradf_.FastPartialDerivative(
198-
minimizer_->getMultiGenFcn(), minimizer_->fitter()->Config().ParamsSettings(), i_component, grad_[i_component]);
205+
grad_[i_component] = gradf_.FastPartialDerivative(minim.GetFCN(), minimizer_->fitter()->Config().ParamsSettings(),
206+
i_component, grad_[i_component]);
199207
}
200208

201209
void LikelihoodGradientJob::calculate_all()

0 commit comments

Comments
 (0)