Skip to content

Commit

Permalink
[Minuit2] Use std::span instead of std::vector const& for parameters
Browse files Browse the repository at this point in the history
Use `std::span` instead of `std::vector const&` for function parameters
in Minuit2.

The motivation is that `std::span` is more general. If the function
takes a `std::vector const&`, the inputs are forced to be allocated on
the heap. So if one wants to call functions with constant size or even
scalar input, that would cause a large overhead.

This overhead can be avoided when generalizing with `std::span`.

The standalone Minuit2 build files were also changed to consider the
`std::span` backport to C++17 that is already in ROOT.
  • Loading branch information
guitargeek committed Sep 9, 2024
1 parent 064217a commit 63636f6
Show file tree
Hide file tree
Showing 63 changed files with 287 additions and 233 deletions.
4 changes: 3 additions & 1 deletion math/mathcore/inc/Math/Minimizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include "Math/IFunction.h"
#include "Math/MinimizerOptions.h"

#include <ROOT/RSpan.hxx>

#include <string>
#include <limits>
#include <cmath>
Expand Down Expand Up @@ -159,7 +161,7 @@ class Minimizer {
virtual void SetFunction(const ROOT::Math::IMultiGenFunction & func) = 0;

/// set the function implementing Hessian computation (re-implemented by Minimizer using it)
virtual void SetHessianFunction(std::function<bool(const std::vector<double> &, double *)> ) {}
virtual void SetHessianFunction(std::function<bool(std::span<const double>, double *)> ) {}

/// add variables . Return number of variables successfully added
template<class VariableIterator>
Expand Down
2 changes: 1 addition & 1 deletion math/mathcore/src/Fitter.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ bool Fitter::DoInitMinimizer() {
const ROOT::Math::FitMethodGradFunction *fitGradFcn =
dynamic_cast<const ROOT::Math::FitMethodGradFunction *>(gradfcn);
if (fitGradFcn && fitGradFcn->HasHessian()) {
auto hessFcn = [=](const std::vector<double> &x, double *hess) {
auto hessFcn = [=](std::span<const double> x, double *hess) {
unsigned int ndim = x.size();
unsigned int nh = ndim * (ndim + 1) / 2;
std::vector<double> h(nh);
Expand Down
10 changes: 10 additions & 0 deletions math/minuit2/StandAlone.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ endfunction()
# This file adds copy_standalone
include(copy_standalone.cmake)

# We need the std::span backport. This can be removed once the minimum C++
# standard gets raised to C++20, which should happen if also the minimum C++
# standard of ROOT gets raised and the std::span backport in RSpan is removed.
include_directories(../../core/foundation/inc/)
set(SPAN_HEADERS RSpan.hxx span.hxx)
copy_standalone(SOURCE ../../core/foundation/inc/ROOT DESTINATION . OUTPUT SPAN_HEADERS
FILES ${SPAN_HEADERS})
install(FILES ${SPAN_HEADERS} DESTINATION include/ROOT)


# Copy these files in if needed
copy_standalone(SOURCE ../../core/foundation/inc/ROOT DESTINATION . OUTPUT VERSION_FILE
FILES RVersion.hxx)
Expand Down
6 changes: 4 additions & 2 deletions math/minuit2/inc/Minuit2/ContoursError.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include "Minuit2/MnConfig.h"
#include "Minuit2/MinosError.h"

#include <ROOT/RSpan.hxx>

#include <vector>
#include <utility>

Expand All @@ -23,9 +25,9 @@ namespace Minuit2 {
class ContoursError {

public:
ContoursError(unsigned int parX, unsigned int parY, const std::vector<std::pair<double, double>> &points,
ContoursError(unsigned int parX, unsigned int parY, std::span<const std::pair<double, double>> points,
const MinosError &xmnos, const MinosError &ymnos, unsigned int nfcn)
: fParX(parX), fParY(parY), fPoints(points), fXMinos(xmnos), fYMinos(ymnos), fNFcn(nfcn)
: fParX(parX), fParY(parY), fPoints(points.begin(), points.end()), fXMinos(xmnos), fYMinos(ymnos), fNFcn(nfcn)
{
}

Expand Down
4 changes: 3 additions & 1 deletion math/minuit2/inc/Minuit2/FCNAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

#include "Minuit2/FCNBase.h"

#include <ROOT/RSpan.hxx>

#include <vector>

namespace ROOT {
Expand All @@ -37,7 +39,7 @@ class FCNAdapter : public FCNBase {

~FCNAdapter() override {}

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

Expand Down
8 changes: 5 additions & 3 deletions math/minuit2/inc/Minuit2/FCNBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@

#include "Minuit2/MnConfig.h"

#include <vector>

#include "Minuit2/GenericFunction.h"

#include <ROOT/RSpan.hxx>

#include <vector>

namespace ROOT {

namespace Minuit2 {
Expand Down Expand Up @@ -69,7 +71,7 @@ class FCNBase : public GenericFunction {
*/

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

/**
Expand Down
14 changes: 7 additions & 7 deletions math/minuit2/inc/Minuit2/FCNGradAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,17 @@ class FCNGradAdapter : public FCNGradientBase {

~FCNGradAdapter() override {}

double operator()(const std::vector<double> &v) const override { return fFunc.operator()(&v[0]); }
double operator()(std::span<const double> 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(const std::vector<double> &v) const override
std::vector<double> Gradient(std::span<const double> v) const override
{
fFunc.Gradient(&v[0], &fGrad[0]);
return fGrad;
}
std::vector<double> GradientWithPrevResult(const std::vector<double> &v, double *previous_grad, double *previous_g2,
std::vector<double> GradientWithPrevResult(std::span<const double> v, double *previous_grad, double *previous_g2,
double *previous_gstep) const override
{
fFunc.GradientWithPrevResult(&v[0], &fGrad[0], previous_grad, previous_g2, previous_gstep);
Expand All @@ -68,7 +68,7 @@ class FCNGradAdapter : public FCNGradientBase {
}

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

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

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

} // end namespace Minuit2
Expand Down
8 changes: 4 additions & 4 deletions math/minuit2/inc/Minuit2/FCNGradientBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ class FCNGradientBase : public FCNBase {
public:
~FCNGradientBase() override {}

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

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

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

virtual bool HasHessian() const { return false; }

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

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

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

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

double chiSquare = 0.0;
Expand Down
8 changes: 4 additions & 4 deletions math/minuit2/inc/Minuit2/FumiliFCNAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,21 @@ class FumiliFCNAdapter : public FumiliFCNBase {

~FumiliFCNAdapter() override {}

double operator()(const std::vector<double> &v) const override { return fFunc.operator()(&v[0]); }
double operator()(std::span<const double> 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;
// virtual std::vector<double> Gradient(std::span<const double> ) 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(const std::vector<double> &v) override;
void EvaluateAll(std::span<const double> v) override;

private:
// data member
Expand All @@ -74,7 +74,7 @@ class FumiliFCNAdapter : public FumiliFCNBase {
};

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

Expand Down
6 changes: 3 additions & 3 deletions math/minuit2/inc/Minuit2/FumiliFCNBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class FumiliFCNBase : public FCNGradientBase {
**/

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

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

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

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

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

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

/**
Expand Down Expand Up @@ -126,7 +126,7 @@ class FumiliMaximumLikelihoodFCN : public FumiliFCNBase {
*/

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

double sumoflogs = 0.0;
Expand Down
21 changes: 12 additions & 9 deletions math/minuit2/inc/Minuit2/FumiliStandardChi2FCN.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

#include "Minuit2/FumiliChi2FCN.h"
#include "Minuit2/ParametricFunction.h"

#include <ROOT/RSpan.hxx>

#include <cassert>
#include <vector>
#include <cmath>
Expand Down Expand Up @@ -57,14 +60,14 @@ class FumiliStandardChi2FCN : public FumiliChi2FCN {
*/

FumiliStandardChi2FCN(const ParametricFunction &modelFCN, const std::vector<double> &meas,
const std::vector<double> &pos, const std::vector<double> &mvar)
FumiliStandardChi2FCN(const ParametricFunction &modelFCN, std::span<const double> meas,
std::span<const double> pos, std::span<const double> mvar)
{ // this->fModelFCN = &modelFunction;
this->SetModelFunction(modelFCN);

assert(meas.size() == pos.size());
assert(meas.size() == mvar.size());
fMeasurements = meas;
fMeasurements.assign(meas.begin(), meas.end());
std::vector<double> x(1);
unsigned int n = mvar.size();
fPositions.reserve(n);
Expand Down Expand Up @@ -98,15 +101,15 @@ class FumiliStandardChi2FCN : public FumiliChi2FCN {
*/

FumiliStandardChi2FCN(const ParametricFunction &modelFCN, const std::vector<double> &meas,
const std::vector<std::vector<double>> &pos, const std::vector<double> &mvar)
FumiliStandardChi2FCN(const ParametricFunction &modelFCN, std::span<const double> meas,
std::span<const std::vector<double>> pos, std::span<const double> mvar)
{ // this->fModelFCN = &modelFunction;
this->SetModelFunction(modelFCN);

assert(meas.size() == pos.size());
assert(meas.size() == mvar.size());
fMeasurements = meas;
fPositions = pos;
fMeasurements.assign(meas.begin(), meas.end());
fPositions.assign(pos.begin(), pos.end());
// correct for variance == 0
unsigned int n = mvar.size();
fInvErrors.resize(n);
Expand Down Expand Up @@ -138,7 +141,7 @@ class FumiliStandardChi2FCN : public FumiliChi2FCN {
*/

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

/**
Expand Down Expand Up @@ -173,7 +176,7 @@ class FumiliStandardChi2FCN : public FumiliChi2FCN {
**/

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

private:
std::vector<double> fMeasurements;
Expand Down
10 changes: 5 additions & 5 deletions math/minuit2/inc/Minuit2/FumiliStandardMaximumLikelihoodFCN.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class FumiliStandardMaximumLikelihoodFCN : public FumiliMaximumLikelihoodFCN {
*/

FumiliStandardMaximumLikelihoodFCN(const ParametricFunction &modelFCN, const std::vector<double> &pos)
FumiliStandardMaximumLikelihoodFCN(const ParametricFunction &modelFCN, std::span<const double> pos)
{
this->SetModelFunction(modelFCN);
unsigned int n = pos.size();
Expand All @@ -68,10 +68,10 @@ class FumiliStandardMaximumLikelihoodFCN : public FumiliMaximumLikelihoodFCN {
*/

FumiliStandardMaximumLikelihoodFCN(const ParametricFunction &modelFCN, const std::vector<std::vector<double>> &pos)
FumiliStandardMaximumLikelihoodFCN(const ParametricFunction &modelFCN, std::span<const std::vector<double>> pos)
{
this->SetModelFunction(modelFCN);
fPositions = pos;
fPositions.assign(pos.begin(), pos.end());
}

~FumiliStandardMaximumLikelihoodFCN() override {}
Expand All @@ -88,7 +88,7 @@ class FumiliStandardMaximumLikelihoodFCN : public FumiliMaximumLikelihoodFCN {
*/

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

/**
Expand Down Expand Up @@ -123,7 +123,7 @@ class FumiliStandardMaximumLikelihoodFCN : public FumiliMaximumLikelihoodFCN {
**/

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

private:
std::vector<std::vector<double>> fPositions;
Expand Down
Loading

0 comments on commit 63636f6

Please sign in to comment.