Skip to content

Commit

Permalink
[Minuit2] Revert recent changes to virtual function signature
Browse files Browse the repository at this point in the history
This reverts part of 63636f6 from a few days ago, which also changed
the interface of virtual functions, which was not the intention and
breaks user code.
guitargeek committed Oct 3, 2024
1 parent 0229d5d commit d8458dd
Showing 26 changed files with 61 additions and 108 deletions.
9 changes: 1 addition & 8 deletions math/minuit2/inc/Minuit2/FCNAdapter.h
Original file line number Diff line number Diff line change
@@ -37,19 +37,12 @@ class FCNAdapter : public FCNBase {
public:
FCNAdapter(const Function &f, double up = 1.) : fFunc(f), fUp(up) {}

~FCNAdapter() override {}

double operator()(std::span<const double> v) const override { return fFunc.operator()(&v[0]); }
double operator()(std::vector<double> const& v) const override { return fFunc.operator()(&v[0]); }
double operator()(const double *v) const { return fFunc.operator()(v); }
double Up() const override { return fUp; }

void SetErrorDef(double up) override { fUp = up; }

// virtual std::vector<double> Gradient(const std::vector<double>&) const;

// forward interface
// virtual double operator()(int npar, double* params,int iflag = 4) const;

private:
const Function &fFunc;
double fUp;
10 changes: 5 additions & 5 deletions math/minuit2/inc/Minuit2/FCNBase.h
Original file line number Diff line number Diff line change
@@ -74,7 +74,7 @@ class FCNBase : public GenericFunction {
*/

double operator()(std::span<const double> v) const override = 0;
double operator()(std::vector<double> const &v) const override = 0;

/**
@@ -112,8 +112,8 @@ class FCNBase : public GenericFunction {

virtual bool HasGradient() const { return false; }

virtual std::vector<double> Gradient(std::span<const double> ) const { return {}; }
virtual std::vector<double> GradientWithPrevResult(std::span<const double> parameters, double * /*previous_grad*/,
virtual std::vector<double> Gradient(std::vector<double> const&) const { return {}; }
virtual std::vector<double> GradientWithPrevResult(std::vector<double> const& parameters, double * /*previous_grad*/,
double * /*previous_g2*/, double * /*previous_gstep*/) const
{
return Gradient(parameters);
@@ -124,10 +124,10 @@ class FCNBase : public GenericFunction {
};

/// return second derivatives (diagonal of the Hessian matrix)
virtual std::vector<double> G2(std::span<const double> ) const { return {};}
virtual std::vector<double> G2(std::vector<double> const&) const { return {};}

/// return Hessian
virtual std::vector<double> Hessian(std::span<const double> ) const { return {};}
virtual std::vector<double> Hessian(std::vector<double> const&) const { return {};}

virtual bool HasHessian() const { return false; }

14 changes: 7 additions & 7 deletions math/minuit2/inc/Minuit2/FCNGradAdapter.h
Original file line number Diff line number Diff line change
@@ -41,17 +41,17 @@ class FCNGradAdapter : public FCNBase {

bool HasGradient() const override { return true; }

double operator()(std::span<const double> v) const override { return fFunc.operator()(&v[0]); }
double operator()(std::vector<double> const& v) const override { return fFunc.operator()(&v[0]); }
double operator()(const double *v) const { return fFunc.operator()(v); }

double Up() const override { return fUp; }

std::vector<double> Gradient(std::span<const double> v) const override
std::vector<double> Gradient(std::vector<double> const& v) const override
{
fFunc.Gradient(&v[0], &fGrad[0]);
return fGrad;
}
std::vector<double> GradientWithPrevResult(std::span<const double> v, double *previous_grad, double *previous_g2,
std::vector<double> GradientWithPrevResult(std::vector<double> const& v, double *previous_grad, double *previous_g2,
double *previous_gstep) const override
{
fFunc.GradientWithPrevResult(&v[0], &fGrad[0], previous_grad, previous_g2, previous_gstep);
@@ -67,7 +67,7 @@ class FCNGradAdapter : public FCNBase {
}

/// return second derivatives (diagonal of the Hessian matrix)
std::vector<double> G2(std::span<const double> x) const override {
std::vector<double> G2(std::vector<double> const& x) const override {
if (fG2Func)
return fG2Func(x);
if (fHessianFunc) {
@@ -88,7 +88,7 @@ class FCNGradAdapter : public FCNBase {
}

/// compute Hessian. Return Hessian as a std::vector of size(n*n)
std::vector<double> Hessian(std::span<const double> x ) const override {
std::vector<double> Hessian(std::vector<double> const& x ) const override {
unsigned int n = fFunc.NDim();
if (fHessianFunc) {
if (fHessian.empty() ) fHessian.resize(n * n);
@@ -126,8 +126,8 @@ class FCNGradAdapter : public FCNBase {
mutable std::vector<double> fHessian;
mutable std::vector<double> fG2Vec;

std::function<std::vector<double>(std::span<const double> )> fG2Func;
mutable std::function<bool(std::span<const double> , double *)> fHessianFunc;
std::function<std::vector<double>(std::vector<double> const& )> fG2Func;
mutable std::function<bool(std::vector<double> const& , double *)> fHessianFunc;
};

} // end namespace Minuit2
4 changes: 2 additions & 2 deletions math/minuit2/inc/Minuit2/FumiliChi2FCN.h
Original file line number Diff line number Diff line change
@@ -87,7 +87,7 @@ class FumiliChi2FCN : public FumiliFCNBase {
*/

virtual std::vector<double> Elements(std::span<const double> par) const = 0;
virtual std::vector<double> Elements(std::vector<double> const &par) const = 0;

/**
@@ -129,7 +129,7 @@ class FumiliChi2FCN : public FumiliFCNBase {
*/

double operator()(std::span<const double> par) const override
double operator()(std::vector<double> const &par) const override
{

double chiSquare = 0.0;
14 changes: 4 additions & 10 deletions math/minuit2/inc/Minuit2/FumiliFCNAdapter.h
Original file line number Diff line number Diff line change
@@ -48,38 +48,32 @@ class FumiliFCNAdapter : public FumiliFCNBase {

FumiliFCNAdapter(const Function &f, unsigned int ndim, double up = 1.) : FumiliFCNBase(ndim), fFunc(f), fUp(up) {}

~FumiliFCNAdapter() override {}

double operator()(std::span<const double> v) const override { return fFunc.operator()(&v[0]); }
double operator()(std::vector<double> const &v) const override { return fFunc.operator()(&v[0]); }
double operator()(const double *v) const { return fFunc.operator()(v); }
double Up() const override { return fUp; }

void SetErrorDef(double up) override { fUp = up; }

// virtual std::vector<double> Gradient(std::span<const double> ) const;
// virtual std::vector<double> Gradient(std::vector<double> const &) const;

// forward interface
// virtual double operator()(int npar, double* params,int iflag = 4) const;

/**
evaluate gradient hessian and function value needed by fumili
*/
void EvaluateAll(std::span<const double> v) override;
void EvaluateAll(std::vector<double> const &v) override;

private:
// data member

const Function &fFunc;
double fUp;
};

template <class Function>
void FumiliFCNAdapter<Function>::EvaluateAll(std::span<const double> v)
void FumiliFCNAdapter<Function>::EvaluateAll(std::vector<double> const &v)
{
MnPrint print("FumiliFCNAdapter");

// typedef FumiliFCNAdapter::Function Function;

// evaluate all elements
unsigned int npar = Dimension();
if (npar != v.size())
6 changes: 3 additions & 3 deletions math/minuit2/inc/Minuit2/FumiliFCNBase.h
Original file line number Diff line number Diff line change
@@ -84,7 +84,7 @@ class FumiliFCNBase : public FCNBase {
**/

virtual void EvaluateAll(std::span<const double> par) = 0;
virtual void EvaluateAll(std::vector<double> const& par) = 0;

/**
Return cached Value of objective function estimated previously using the FumiliFCNBase::EvaluateAll method
@@ -98,7 +98,7 @@ class FumiliFCNBase : public FCNBase {
**/

virtual const std::vector<double> &Gradient() const { return fGradient; }
std::vector<double> Gradient(std::span<const double>) const override { return fGradient;}
std::vector<double> Gradient(std::vector<double> const&) const override { return fGradient;}

/**
Return Value of the i-th j-th element of the Hessian matrix estimated previously using the
@@ -107,7 +107,7 @@ class FumiliFCNBase : public FCNBase {
@param col col Index of the matrix
**/

std::vector<double> Hessian(std::span<const double>) const override { return fHessian;}
std::vector<double> Hessian(std::vector<double> const&) const override { return fHessian;}
virtual double Hessian(unsigned int row, unsigned int col) const
{
assert(row < fGradient.size() && col < fGradient.size());
6 changes: 2 additions & 4 deletions math/minuit2/inc/Minuit2/FumiliMaximumLikelihoodFCN.h
Original file line number Diff line number Diff line change
@@ -48,8 +48,6 @@ class FumiliMaximumLikelihoodFCN : public FumiliFCNBase {
public:
FumiliMaximumLikelihoodFCN() {}

~FumiliMaximumLikelihoodFCN() override {}

/**
Sets the model function for the data (for example gaussian+linear for a peak)
@@ -84,7 +82,7 @@ class FumiliMaximumLikelihoodFCN : public FumiliFCNBase {
*/

virtual std::vector<double> Elements(std::span<const double> par) const = 0;
virtual std::vector<double> Elements(std::vector<double> const &par) const = 0;

/**
@@ -126,7 +124,7 @@ class FumiliMaximumLikelihoodFCN : public FumiliFCNBase {
*/

double operator()(std::span<const double> par) const override
double operator()(std::vector<double> const &par) const override
{

double sumoflogs = 0.0;
6 changes: 2 additions & 4 deletions math/minuit2/inc/Minuit2/FumiliStandardChi2FCN.h
Original file line number Diff line number Diff line change
@@ -122,8 +122,6 @@ class FumiliStandardChi2FCN : public FumiliChi2FCN {
}
}

~FumiliStandardChi2FCN() override {}

/**
Evaluates the model function for the different measurement points and
@@ -141,7 +139,7 @@ class FumiliStandardChi2FCN : public FumiliChi2FCN {
*/

std::vector<double> Elements(std::span<const double> par) const override;
std::vector<double> Elements(std::vector<double> const &par) const override;

/**
@@ -176,7 +174,7 @@ class FumiliStandardChi2FCN : public FumiliChi2FCN {
**/

void EvaluateAll(std::span<const double> par) override;
void EvaluateAll(std::vector<double> const &par) override;

private:
std::vector<double> fMeasurements;
6 changes: 2 additions & 4 deletions math/minuit2/inc/Minuit2/FumiliStandardMaximumLikelihoodFCN.h
Original file line number Diff line number Diff line change
@@ -74,8 +74,6 @@ class FumiliStandardMaximumLikelihoodFCN : public FumiliMaximumLikelihoodFCN {
fPositions.assign(pos.begin(), pos.end());
}

~FumiliStandardMaximumLikelihoodFCN() override {}

/**
Evaluates the model function for the different measurement points and
@@ -88,7 +86,7 @@ class FumiliStandardMaximumLikelihoodFCN : public FumiliMaximumLikelihoodFCN {
*/

std::vector<double> Elements(std::span<const double> par) const override;
std::vector<double> Elements(std::vector<double> const &par) const override;

/**
@@ -123,7 +121,7 @@ class FumiliStandardMaximumLikelihoodFCN : public FumiliMaximumLikelihoodFCN {
**/

void EvaluateAll(std::span<const double> par) override;
void EvaluateAll(std::vector<double> const &par) override;

private:
std::vector<std::vector<double>> fPositions;
2 changes: 1 addition & 1 deletion math/minuit2/inc/Minuit2/GenericFunction.h
Original file line number Diff line number Diff line change
@@ -49,7 +49,7 @@ class GenericFunction {
*/

virtual double operator()(std::span<const double> x) const = 0;
virtual double operator()(std::vector<double> const& x) const = 0;
};

} // namespace Minuit2
10 changes: 4 additions & 6 deletions math/minuit2/inc/Minuit2/ParametricFunction.h
Original file line number Diff line number Diff line change
@@ -65,8 +65,6 @@ class ParametricFunction : public FCNBase {

ParametricFunction(int nparams) : par(nparams) {}

~ParametricFunction() override {}

/**
Sets the parameters of the ParametricFunction.
@@ -75,7 +73,7 @@ class ParametricFunction : public FCNBase {
*/

virtual void SetParameters(std::span<const double> params) const
virtual void SetParameters(std::vector<double> const& params) const
{

assert(params.size() == par.size());
@@ -115,7 +113,7 @@ class ParametricFunction : public FCNBase {
*/

double operator()(std::span<const double> x) const override = 0;
double operator()(std::vector<double> const& x) const override = 0;

/**
@@ -134,7 +132,7 @@ class ParametricFunction : public FCNBase {
*/

virtual double operator()(std::span<const double> x, std::span<const double> params) const
virtual double operator()(std::vector<double> const& x, std::vector<double> const& params) const
{
SetParameters(params);
return operator()(x);
@@ -153,7 +151,7 @@ class ParametricFunction : public FCNBase {
*/

virtual std::vector<double> GetGradient(std::span<const double> x) const;
virtual std::vector<double> GetGradient(std::vector<double> const& x) const;

protected:
/**
4 changes: 2 additions & 2 deletions math/minuit2/src/FumiliStandardChi2FCN.cxx
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ namespace ROOT {

namespace Minuit2 {

std::vector<double> FumiliStandardChi2FCN::Elements(std::span<const double> par) const
std::vector<double> FumiliStandardChi2FCN::Elements(std::vector<double> const &par) const
{
// Calculate the f(i) contribution to the Chi2. Chi2 = Sum[f(i)**2]

@@ -54,7 +54,7 @@ int FumiliStandardChi2FCN::GetNumberOfMeasurements() const
return fPositions.size();
}

void FumiliStandardChi2FCN::EvaluateAll(std::span<const double> par)
void FumiliStandardChi2FCN::EvaluateAll(std::vector<double> const &par)
{
// Evaluate chi2 value, gradient and hessian all in a single
// loop on the measurements
4 changes: 2 additions & 2 deletions math/minuit2/src/FumiliStandardMaximumLikelihoodFCN.cxx
Original file line number Diff line number Diff line change
@@ -17,7 +17,7 @@ namespace ROOT {

namespace Minuit2 {

std::vector<double> FumiliStandardMaximumLikelihoodFCN::Elements(std::span<const double> par) const
std::vector<double> FumiliStandardMaximumLikelihoodFCN::Elements(std::vector<double> const &par) const
{

// calculate likelihood element f(i) = pdf(x(i))
@@ -54,7 +54,7 @@ int FumiliStandardMaximumLikelihoodFCN::GetNumberOfMeasurements() const
return fPositions.size();
}

void FumiliStandardMaximumLikelihoodFCN::EvaluateAll(std::span<const double> par)
void FumiliStandardMaximumLikelihoodFCN::EvaluateAll(std::vector<double> const &par)
{
// Evaluate in one loop likelihood value, gradient and hessian

4 changes: 1 addition & 3 deletions math/minuit2/src/ParametricFunction.cxx
Original file line number Diff line number Diff line change
@@ -19,9 +19,7 @@ namespace ROOT {

namespace Minuit2 {

//#include "Minuit2/MnPrint.h"

std::vector<double> ParametricFunction::GetGradient(std::span<const double> x) const
std::vector<double> ParametricFunction::GetGradient(std::vector<double> const &x) const
{
// calculate the numerical gradient (using Numerical2PGradientCalculator)

2 changes: 1 addition & 1 deletion math/minuit2/test/MnSim/GaussFcn.cxx
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ namespace ROOT {

namespace Minuit2 {

double GaussFcn::operator()(std::span<const double> par) const
double GaussFcn::operator()(std::vector<double> const &par) const
{

assert(par.size() == 3);
Loading

0 comments on commit d8458dd

Please sign in to comment.