Skip to content

Commit

Permalink
[Minuit2] Reduce boilerplate code by removing redundant overloads
Browse files Browse the repository at this point in the history
In the unlikeliy case that users were relying on these functions, they
can easily migrate to the overload that takes `MnUserParameterState`
without any further overhead just placing some curly brackets. Since
this overload also exists in older ROOT versions, this is a backwards
compatible change on the user side that helps us to keep Minuit 2 code
clean.
  • Loading branch information
guitargeek committed Sep 16, 2024
1 parent 8dd7784 commit c443317
Show file tree
Hide file tree
Showing 22 changed files with 20 additions and 325 deletions.
1 change: 1 addition & 0 deletions README/ReleaseNotes/v634/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ The following people have contributed to this new version:
- The `RooAbsReal::plotSliceOn()` function that was deprecated since at least ROOT 6 was removed. Use `plotOn(frame,Slice(...))` instead.
- The `RooTemplateProxy` constructors that take a `proxyOwnsArg` parameter to manually pass ownership are deprecated and replaced by a new constructor that takes ownership via `std::unique_ptr<T>`. They will be removed in ROOT 6.36.
- Several RooFit legacy functions are deprecated and will be removed in ROOT 6.36 (see section "RooFit libraries")
- Multiple overloads of internal Minuit 2 constructors and functions have been removed. If your code fails to compile, you can easily change to another overload that takes a `MnUserParameterState`, which is a change backwards compatible with older ROOT versions.

## Core Libraries

Expand Down
1 change: 0 additions & 1 deletion math/minuit2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ if(CMAKE_PROJECT_NAME STREQUAL ROOT)
Minuit2/FumiliStandardChi2FCN.h
Minuit2/FumiliStandardMaximumLikelihoodFCN.h
Minuit2/FunctionGradient.h
Minuit2/FunctionMinimizer.h
Minuit2/FunctionMinimum.h
Minuit2/GenericFunction.h
Minuit2/GradientCalculator.h
Expand Down
1 change: 0 additions & 1 deletion math/minuit2/inc/LinkDef.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
#pragma link C++ class ROOT::Minuit2::MnUserParameterState;
#pragma link C++ class ROOT::Minuit2::MnUserParameters;
#pragma link C++ class ROOT::Minuit2::MnStrategy;
#pragma link C++ class ROOT::Minuit2::FunctionMinimizer;
#pragma link C++ class ROOT::Minuit2::ModularFunctionMinimizer;
#pragma link C++ class ROOT::Minuit2::VariableMetricMinimizer;
#pragma link C++ class ROOT::Minuit2::SimplexMinimizer;
Expand Down
2 changes: 0 additions & 2 deletions math/minuit2/inc/Minuit2/ContoursError.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ class ContoursError {
{
}

~ContoursError() {}

ContoursError(const ContoursError &cont)
: fParX(cont.fParX), fParY(cont.fParY), fPoints(cont.fPoints), fXMinos(cont.fXMinos), fYMinos(cont.fYMinos),
fNFcn(cont.fNFcn)
Expand Down
2 changes: 0 additions & 2 deletions math/minuit2/inc/Minuit2/FumiliMinimizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ class FumiliMinimizer : public ModularFunctionMinimizer {

FumiliMinimizer() : fMinSeedGen(MnSeedGenerator()), fMinBuilder(FumiliBuilder()) {}

~FumiliMinimizer() override {}

/**
Accessor to the seed generator of the minimizer.
Expand Down
55 changes: 0 additions & 55 deletions math/minuit2/inc/Minuit2/FunctionMinimizer.h

This file was deleted.

29 changes: 0 additions & 29 deletions math/minuit2/inc/Minuit2/MnFumiliMinimize.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,35 +38,6 @@ also used by MnMinos and MnContours;
class MnFumiliMinimize : public MnApplication {

public:
/// construct from FumiliFCNBase + std::vector for parameters and errors
MnFumiliMinimize(const FumiliFCNBase &fcn, std::span<const double> par, std::span<const double> err,
unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, err), MnStrategy(stra)), fMinimizer(FumiliMinimizer()), fFCN(fcn)
{
}

/// construct from FumiliFCNBase + std::vector for parameters and covariance
MnFumiliMinimize(const FumiliFCNBase &fcn, std::span<const double> par, unsigned int nrow,
std::span<const double> cov, unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, cov, nrow), MnStrategy(stra)), fMinimizer(FumiliMinimizer()),
fFCN(fcn)
{
}

/// construct from FumiliFCNBase + std::vector for parameters and MnUserCovariance
MnFumiliMinimize(const FumiliFCNBase &fcn, std::span<const double> par, const MnUserCovariance &cov,
unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, cov), MnStrategy(stra)), fMinimizer(FumiliMinimizer()), fFCN(fcn)
{
}

/// construct from FumiliFCNBase + MnUserParameters + MnUserCovariance
MnFumiliMinimize(const FumiliFCNBase &fcn, const MnUserParameters &par, const MnUserCovariance &cov,
unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, cov), MnStrategy(stra)), fMinimizer(FumiliMinimizer()), fFCN(fcn)
{
}

/// construct from FumiliFCNBase + MnUserParameterState + MnStrategy
MnFumiliMinimize(const FumiliFCNBase &fcn, const MnUserParameterState &par, const MnStrategy &str = MnStrategy{1})
: MnApplication(fcn, MnUserParameterState(par), str), fMinimizer(FumiliMinimizer()), fFCN(fcn)
Expand Down
18 changes: 0 additions & 18 deletions math/minuit2/inc/Minuit2/MnHesse.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,24 +50,6 @@ class MnHesse {
/// conctructor with specific strategy
MnHesse(const MnStrategy &stra) : fStrategy(stra) {}

///
/// low-level API
///
/// FCN + parameters + errors
MnUserParameterState operator()(const FCNBase &, std::span<const double>, std::span<const double>,
unsigned int maxcalls = 0) const;
/// FCN + parameters + covariance
MnUserParameterState operator()(const FCNBase &, std::span<const double>, unsigned int nrow,
std::span<const double>, unsigned int maxcalls = 0) const;
/// FCN + parameters + MnUserCovariance
MnUserParameterState
operator()(const FCNBase &, std::span<const double>, const MnUserCovariance &, unsigned int maxcalls = 0) const;
///
/// high-level API
///
/// FCN + MnUserParameters + MnUserCovariance
MnUserParameterState
operator()(const FCNBase &, const MnUserParameters &, const MnUserCovariance &, unsigned int maxcalls = 0) const;
/// FCN + MnUserParameterState
MnUserParameterState operator()(const FCNBase &, const MnUserParameterState &, unsigned int maxcalls = 0) const;
///
Expand Down
26 changes: 0 additions & 26 deletions math/minuit2/inc/Minuit2/MnMigrad.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,32 +32,6 @@ class FCNBase;
class MnMigrad : public MnApplication {

public:
/// construct from FCNBase + std::vector for parameters and errors
MnMigrad(const FCNBase &fcn, std::span<const double> par, std::span<const double> err, unsigned int stra = 1)
: MnApplication(fcn, {par, err}, MnStrategy(stra)), fMinimizer(VariableMetricMinimizer())
{
}

/// construct from FCNBase + std::vector for parameters and covariance
MnMigrad(const FCNBase &fcn, std::span<const double> par, unsigned int nrow, std::span<const double> cov,
unsigned int stra = 1)
: MnApplication(fcn, {par, cov, nrow}, MnStrategy(stra)),
fMinimizer(VariableMetricMinimizer())
{
}

/// construct from FCNBase + std::vector for parameters and MnUserCovariance
MnMigrad(const FCNBase &fcn, std::span<const double> par, const MnUserCovariance &cov, unsigned int stra = 1)
: MnApplication(fcn, {par, cov}, MnStrategy(stra)), fMinimizer(VariableMetricMinimizer())
{
}

/// construct from FCNBase + MnUserParameters + MnUserCovariance
MnMigrad(const FCNBase &fcn, const MnUserParameters &par, const MnUserCovariance &cov, unsigned int stra = 1)
: MnApplication(fcn, {par, cov}, MnStrategy(stra)), fMinimizer(VariableMetricMinimizer())
{
}

/// construct from FCNBase + MnUserParameterState + MnStrategy
MnMigrad(const FCNBase &fcn, const MnUserParameterState &par, const MnStrategy &str = MnStrategy{1})
: MnApplication(fcn, {par}, str), fMinimizer(VariableMetricMinimizer())
Expand Down
25 changes: 0 additions & 25 deletions math/minuit2/inc/Minuit2/MnMinimize.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,31 +30,6 @@ class FCNBase;
class MnMinimize : public MnApplication {

public:
/// construct from FCNBase + std::vector for parameters and errors
MnMinimize(const FCNBase &fcn, std::span<const double> par, std::span<const double> err, unsigned int stra = 1)
: MnApplication(fcn, {par, err}, MnStrategy(stra)), fMinimizer(CombinedMinimizer())
{
}

/// construct from FCNBase + std::vector for parameters and covariance
MnMinimize(const FCNBase &fcn, std::span<const double> par, unsigned int nrow, std::span<const double> cov,
unsigned int stra = 1)
: MnApplication(fcn, {par, cov, nrow}, MnStrategy(stra)), fMinimizer(CombinedMinimizer())
{
}

/// construct from FCNBase + std::vector for parameters and MnUserCovariance
MnMinimize(const FCNBase &fcn, std::span<const double> par, const MnUserCovariance &cov, unsigned int stra = 1)
: MnApplication(fcn, {par, cov}, MnStrategy(stra)), fMinimizer(CombinedMinimizer())
{
}

/// construct from FCNBase + MnUserParameters + MnUserCovariance
MnMinimize(const FCNBase &fcn, const MnUserParameters &par, const MnUserCovariance &cov, unsigned int stra = 1)
: MnApplication(fcn, {par, cov}, MnStrategy(stra)), fMinimizer(CombinedMinimizer())
{
}

/// construct from FCNBase + MnUserParameterState + MnStrategy
MnMinimize(const FCNBase &fcn, const MnUserParameterState &par, const MnStrategy &str = MnStrategy{1})
: MnApplication(fcn, {par}, str), fMinimizer(CombinedMinimizer())
Expand Down
26 changes: 0 additions & 26 deletions math/minuit2/inc/Minuit2/MnScan.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,31 +33,6 @@ class FCNBase;
class MnScan : public MnApplication {

public:
/// construct from FCNBase + std::vector for parameters and errors
MnScan(const FCNBase &fcn, std::span<const double> par, std::span<const double> err, unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, err), MnStrategy(stra)), fMinimizer(ScanMinimizer())
{
}

/// construct from FCNBase + std::vector for parameters and covariance
MnScan(const FCNBase &fcn, std::span<const double> par, unsigned int nrow, std::span<const double> cov,
unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, cov, nrow), MnStrategy(stra)), fMinimizer(ScanMinimizer())
{
}

/// construct from FCNBase + std::vector for parameters and MnUserCovariance
MnScan(const FCNBase &fcn, std::span<const double> par, const MnUserCovariance &cov, unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, cov), MnStrategy(stra)), fMinimizer(ScanMinimizer())
{
}

/// construct from FCNBase + MnUserParameters + MnUserCovariance
MnScan(const FCNBase &fcn, const MnUserParameters &par, const MnUserCovariance &cov, unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, cov), MnStrategy(stra)), fMinimizer(ScanMinimizer())
{
}

/// construct from FCNBase + MnUserParameterState + MnStrategy
MnScan(const FCNBase &fcn, const MnUserParameterState &par, const MnStrategy &str = MnStrategy{})
: MnApplication(fcn, MnUserParameterState(par), str), fMinimizer(ScanMinimizer())
Expand All @@ -78,7 +53,6 @@ class MnScan : public MnApplication {
private:
ScanMinimizer fMinimizer;

private:
/// forbidden assignment (const FCNBase& = )
MnScan &operator=(const MnScan &) { return *this; }
};
Expand Down
25 changes: 0 additions & 25 deletions math/minuit2/inc/Minuit2/MnSimplex.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,31 +35,6 @@ class FCNBase;
class MnSimplex : public MnApplication {

public:
/// construct from FCNBase + std::vector for parameters and errors
MnSimplex(const FCNBase &fcn, std::span<const double> par, std::span<const double> err, unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, err), MnStrategy(stra)), fMinimizer(SimplexMinimizer())
{
}

/// construct from FCNBase + std::vector for parameters and covariance
MnSimplex(const FCNBase &fcn, std::span<const double> par, unsigned int nrow, std::span<const double> cov,
unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, cov, nrow), MnStrategy(stra)), fMinimizer(SimplexMinimizer())
{
}

/// construct from FCNBase + std::vector for parameters and MnUserCovariance
MnSimplex(const FCNBase &fcn, std::span<const double> par, const MnUserCovariance &cov, unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, cov), MnStrategy(stra)), fMinimizer(SimplexMinimizer())
{
}

/// construct from FCNBase + MnUserParameters + MnUserCovariance
MnSimplex(const FCNBase &fcn, const MnUserParameters &par, const MnUserCovariance &cov, unsigned int stra = 1)
: MnApplication(fcn, MnUserParameterState(par, cov), MnStrategy(stra)), fMinimizer(SimplexMinimizer())
{
}

/// construct from FCNBase + MnUserParameterState + MnStrategy
MnSimplex(const FCNBase &fcn, const MnUserParameterState &par, const MnStrategy &str = MnStrategy{1})
: MnApplication(fcn, MnUserParameterState(par), str), fMinimizer(SimplexMinimizer())
Expand Down
17 changes: 2 additions & 15 deletions math/minuit2/inc/Minuit2/MnUserCovariance.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace Minuit2 {
class MnUserCovariance {

public:
MnUserCovariance() : fData(std::vector<double>()), fNRow(0) {}
MnUserCovariance() = default;

// safe constructor using std::vector
MnUserCovariance(std::span<const double> data, unsigned int nrow) : fData(data.begin(), data.end()), fNRow(nrow)
Expand All @@ -45,19 +45,6 @@ class MnUserCovariance {

MnUserCovariance(unsigned int n) : fData(std::vector<double>(n * (n + 1) / 2, 0.)), fNRow(n) {}

~MnUserCovariance() {}

MnUserCovariance(const MnUserCovariance &cov) : fData(cov.fData), fNRow(cov.fNRow) {}

MnUserCovariance &operator=(const MnUserCovariance &cov)
{
if (this != &cov) {
fData = cov.fData;
fNRow = cov.fNRow;
}
return *this;
}

double operator()(unsigned int row, unsigned int col) const
{
assert(row < fNRow && col < fNRow);
Expand Down Expand Up @@ -91,7 +78,7 @@ class MnUserCovariance {

private:
std::vector<double> fData;
unsigned int fNRow;
unsigned int fNRow = 0;
};

} // namespace Minuit2
Expand Down
34 changes: 6 additions & 28 deletions math/minuit2/inc/Minuit2/ModularFunctionMinimizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

#include "Minuit2/MnConfig.h"

#include "Minuit2/FunctionMinimizer.h"
#include "Minuit2/MnStrategy.h"

#include <vector>

Expand All @@ -28,47 +28,25 @@ class GradientCalculator;
class MnUserParameterState;
class MnUserParameters;
class MnUserCovariance;
class MnStrategy;
class FCNBase;
class FumiliFCNBase;
class FunctionMinimum;

//_____________________________________________________________
/**
Base common class providing the API for all the minimizer
Various Minimize methods are provided varying on the type of
FCN function passesd and on the objects used for the parameters
*/
class ModularFunctionMinimizer : public FunctionMinimizer {
class ModularFunctionMinimizer {

public:
// inherited interface
FunctionMinimum Minimize(const FCNBase &, std::span<const double>, std::span<const double>,
unsigned int stra = 1, unsigned int maxfcn = 0, double toler = 0.1) const override;

FunctionMinimum Minimize(const FCNBase &, std::span<const double>, unsigned int,
std::span<const double>, unsigned int stra = 1, unsigned int maxfcn = 0,
double toler = 0.1) const override;
virtual ~ModularFunctionMinimizer() = default;

// extension
virtual FunctionMinimum Minimize(const FCNBase &, const MnUserParameters &, const MnUserCovariance &,
const MnStrategy &, unsigned int maxfcn = 0, double toler = 0.1) const;

virtual FunctionMinimum Minimize(const FCNBase &, const MnUserParameterState &, const MnStrategy &,
virtual FunctionMinimum Minimize(const FCNBase &, const MnUserParameterState &, const MnStrategy & = MnStrategy{1},
unsigned int maxfcn = 0, double toler = 0.1) const;

// for Fumili

// virtual FunctionMinimum Minimize(const FumiliFCNBase&, const std::vector<double>&, const std::vector<double>&,
// unsigned int stra=1, unsigned int maxfcn = 0, double toler = 0.1) const;

// virtual FunctionMinimum Minimize(const FumiliFCNBase&, const MnUserParameters&, const MnStrategy&, unsigned int
// maxfcn = 0, double toler = 0.1) const;

// virtual FunctionMinimum Minimize(const FumiliFCNBase&, const MnUserParameters&, const MnUserCovariance&, const
// MnStrategy&, unsigned int maxfcn = 0, double toler = 0.1) const;

// virtual FunctionMinimum Minimize(const FumiliFCNBase&, const MnUserParameterState&, const MnStrategy&, unsigned
// int maxfcn = 0, double toler = 0.1) const;

virtual const MinimumSeedGenerator &SeedGenerator() const = 0;
virtual const MinimumBuilder &Builder() const = 0;
virtual MinimumBuilder &Builder() = 0;
Expand Down
Loading

0 comments on commit c443317

Please sign in to comment.