[Minuit2] Add virtual FCNBase::HasGradient() method

By introducing a virtual `FCNBase::HasGradient()` method and moving the former `FCNGradientBase` interface to the base class, we don't need to repeat lots of code just for dealing with both the `FCNBase` and `FCNGradientBase` types. This is a completely backwards compatible change that makes the Minuit 2 code more maintainable.
root-project · Sep 14, 2024 · 71806b3 · 71806b3
1 parent 94549da
commit 71806b3
Show file tree

Hide file tree

Showing 20 changed files with 96 additions and 294 deletions.
diff --git a/math/minuit2/inc/Minuit2/AnalyticalGradientCalculator.h b/math/minuit2/inc/Minuit2/AnalyticalGradientCalculator.h
@@ -17,14 +17,14 @@ namespace ROOT {
 
 namespace Minuit2 {
 
-class FCNGradientBase;
+class FCNBase;
 class MnUserTransformation;
 
 
 class AnalyticalGradientCalculator : public GradientCalculator {
 
 public:
-   AnalyticalGradientCalculator(const FCNGradientBase &fcn, const MnUserTransformation &state)
+   AnalyticalGradientCalculator(const FCNBase &fcn, const MnUserTransformation &state)
       : fGradFunc(fcn), fTransformation(state)
    {
    }
@@ -46,7 +46,7 @@ class AnalyticalGradientCalculator : public GradientCalculator {
    virtual bool CanComputeHessian() const;
 
 protected:
-   const FCNGradientBase &fGradFunc;
+   const FCNBase &fGradFunc;
    const MnUserTransformation &fTransformation;
 };
 

diff --git a/math/minuit2/inc/Minuit2/ExternalInternalGradientCalculator.h b/math/minuit2/inc/Minuit2/ExternalInternalGradientCalculator.h
@@ -16,7 +16,7 @@ namespace ROOT {
 
 namespace Minuit2 {
 
-class FCNGradientBase;
+class FCNBase;
 class MnUserTransformation;
 
 /// Similar to the AnalyticalGradientCalculator, the ExternalInternalGradientCalculator
@@ -30,7 +30,7 @@ class MnUserTransformation;
 class ExternalInternalGradientCalculator : public AnalyticalGradientCalculator {
 
 public:
-   ExternalInternalGradientCalculator(const FCNGradientBase &fcn, const MnUserTransformation &trafo)
+   ExternalInternalGradientCalculator(const FCNBase &fcn, const MnUserTransformation &trafo)
       : AnalyticalGradientCalculator(fcn, trafo)
    {
    }

diff --git a/math/minuit2/inc/Minuit2/FCNBase.h b/math/minuit2/inc/Minuit2/FCNBase.h
@@ -32,6 +32,10 @@ namespace Minuit2 {
   \ingroup Math
 */
 
+enum class GradientParameterSpace {
+  External, Internal
+};
+
 //______________________________________________________________________________
 /**
 
@@ -47,7 +51,6 @@ Interface (abstract class) defining the function to be minimized, which has to b
 class FCNBase : public GenericFunction {
 
 public:
-   ~FCNBase() override {}
 
    /**
 
@@ -106,6 +109,29 @@ class FCNBase : public GenericFunction {
        Re-implement this function if needed.
    */
    virtual void SetErrorDef(double){};
+
+   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*/,
+                                                      double * /*previous_g2*/, double * /*previous_gstep*/) const
+   {
+      return Gradient(parameters);
+   };
+
+   virtual GradientParameterSpace gradParameterSpace() const {
+      return GradientParameterSpace::External;
+   };
+
+   /// return second derivatives (diagonal of the Hessian matrix)
+   virtual std::vector<double> G2(std::span<const double> ) const { return {};}
+
+   /// return Hessian
+   virtual std::vector<double> Hessian(std::span<const double> ) const { return {};}
+
+   virtual bool HasHessian() const { return false; }
+
+   virtual bool HasG2() const { return false; }
 };
 
 } // namespace Minuit2

diff --git a/math/minuit2/inc/Minuit2/FCNGradAdapter.h b/math/minuit2/inc/Minuit2/FCNGradAdapter.h
@@ -10,7 +10,7 @@
 #ifndef ROOT_Minuit2_FCNGradAdapter
 #define ROOT_Minuit2_FCNGradAdapter
 
-#include "Minuit2/FCNGradientBase.h"
+#include "Minuit2/FCNBase.h"
 #include "Minuit2/MnPrint.h"
 
 #include <vector>
@@ -32,13 +32,15 @@ template wrapped class for adapting to FCNBase signature a IGradFunction
 */
 
 template <class Function>
-class FCNGradAdapter : public FCNGradientBase {
+class FCNGradAdapter : public FCNBase {
 
 public:
    FCNGradAdapter(const Function &f, double up = 1.) : fFunc(f), fUp(up), fGrad(std::vector<double>(fFunc.NDim())) {}
 
    ~FCNGradAdapter() override {}
 
+   bool HasGradient() const override { return true; }
+
    double operator()(std::span<const double> v) const override { return fFunc.operator()(&v[0]); }
    double operator()(const double *v) const { return fFunc.operator()(v); }
 

diff --git a/math/minuit2/inc/Minuit2/FCNGradientBase.h b/math/minuit2/inc/Minuit2/FCNGradientBase.h
@@ -12,8 +12,6 @@
 
 #include "Minuit2/FCNBase.h"
 
-#include <vector>
-
 namespace ROOT {
 
 namespace Minuit2 {
@@ -25,37 +23,9 @@ namespace Minuit2 {
     input Parameter vector.
  */
 
-enum class GradientParameterSpace {
-  External, Internal
-};
-
 class FCNGradientBase : public FCNBase {
-
 public:
-   ~FCNGradientBase() override {}
-
-   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);
-   };
-
-   virtual GradientParameterSpace gradParameterSpace() const {
-      return GradientParameterSpace::External;
-   };
-
-   /// return second derivatives (diagonal of the Hessian matrix)
-   virtual std::vector<double> G2(std::span<const double> ) const { return std::vector<double>();}
-
-   /// return Hessian
-   virtual std::vector<double> Hessian(std::span<const double> ) const { return std::vector<double>();}
-
-   virtual bool HasHessian() const { return false; }
-
-   virtual bool HasG2() const { return false; }
-
-
+   bool HasGradient() const final { return true; }
 };
 
 } // namespace Minuit2

diff --git a/math/minuit2/inc/Minuit2/FumiliFCNBase.h b/math/minuit2/inc/Minuit2/FumiliFCNBase.h
@@ -10,7 +10,7 @@
 #ifndef ROOT_Minuit2_FumiliFCNBase
 #define ROOT_Minuit2_FumiliFCNBase
 
-#include "Minuit2/FCNGradientBase.h"
+#include "Minuit2/FCNBase.h"
 #include <cassert>
 #include <vector>
 
@@ -43,7 +43,7 @@ section 5
 
  */
 
-class FumiliFCNBase : public FCNGradientBase {
+class FumiliFCNBase : public FCNBase {
 
 public:
    /**
@@ -53,6 +53,8 @@ class FumiliFCNBase : public FCNGradientBase {
 
    FumiliFCNBase() : fNumberOfParameters(0), fValue(0) {}
 
+   bool HasGradient() const override { return true; }
+
    /**
 
       Constructor which initializes the class with the function provided by the

diff --git a/math/minuit2/inc/Minuit2/FumiliMinimizer.h b/math/minuit2/inc/Minuit2/FumiliMinimizer.h
@@ -88,9 +88,6 @@ class FumiliMinimizer : public ModularFunctionMinimizer {
    FunctionMinimum Minimize(const FCNBase &, const MnUserParameterState &, const MnStrategy &, unsigned int maxfcn = 0,
                             double toler = 0.1) const override;
 
-   FunctionMinimum Minimize(const FCNGradientBase &, const MnUserParameterState &, const MnStrategy &,
-                                    unsigned int maxfcn = 0, double toler = 0.1) const override;
-
    using ModularFunctionMinimizer::Minimize;
 
 private:

diff --git a/math/minuit2/inc/Minuit2/FunctionMinimizer.h b/math/minuit2/inc/Minuit2/FunctionMinimizer.h
@@ -21,7 +21,6 @@ namespace ROOT {
 namespace Minuit2 {
 
 class FCNBase;
-class FCNGradientBase;
 class FunctionMinimum;
 
 //_____________________________________________________________________________________
@@ -43,20 +42,10 @@ class FunctionMinimizer {
    virtual FunctionMinimum Minimize(const FCNBase &, std::span<const double> par, std::span<const double> err,
                                     unsigned int strategy, unsigned int maxfcn, double toler) const = 0;
 
-   // starting values for parameters and errors and FCN with Gradient
-   virtual FunctionMinimum Minimize(const FCNGradientBase &, std::span<const double> par,
-                                    std::span<const double> err, unsigned int strategy, unsigned int maxfcn,
-                                    double toler) const = 0;
-
    // starting values for parameters and covariance matrix
    virtual FunctionMinimum Minimize(const FCNBase &, std::span<const double> par, unsigned int nrow,
                                     std::span<const double> cov, unsigned int strategy, unsigned int maxfcn,
                                     double toler) const = 0;
-
-   // starting values for parameters and covariance matrix and FCN with Gradient
-   virtual FunctionMinimum Minimize(const FCNGradientBase &, std::span<const double> par, unsigned int nrow,
-                                    std::span<const double> cov, unsigned int strategy, unsigned int maxfcn,
-                                    double toler) const = 0;
 };
 
 } // namespace Minuit2

diff --git a/math/minuit2/inc/Minuit2/MnApplication.h b/math/minuit2/inc/Minuit2/MnApplication.h
@@ -24,7 +24,6 @@ class MinuitParameter;
 class MnMachinePrecision;
 class ModularFunctionMinimizer;
 class FCNBase;
-class FCNGradientBase;
 
 //___________________________________________________________________________
 /**
@@ -40,10 +39,6 @@ class MnApplication {
    /// constructor from non-gradient functions
    MnApplication(const FCNBase &fcn, const MnUserParameterState &state, const MnStrategy &stra, unsigned int nfcn = 0);
 
-   /// constructor from gradient function
-   MnApplication(const FCNGradientBase &fcn, const MnUserParameterState &state, const MnStrategy &stra,
-                 unsigned int nfcn = 0);
-
    virtual ~MnApplication() {}
 
    /**
@@ -73,7 +68,6 @@ class MnApplication {
    MnUserParameterState fState;
    MnStrategy fStrategy;
    unsigned int fNumCall;
-   bool fUseGrad;
 
 public:
    // facade: forward interface of MnUserParameters and MnUserTransformation

diff --git a/math/minuit2/inc/Minuit2/MnHesse.h b/math/minuit2/inc/Minuit2/MnHesse.h
@@ -30,7 +30,6 @@ class MinimumState;
 class MnMachinePrecision;
 class MnFcn;
 class FunctionMinimum;
-class FCNGradientBase;
 
 //_______________________________________________________________________
 /**
@@ -97,8 +96,8 @@ class MnHesse {
    /// internal function to compute the Hessian using numerical derivative computation
    MinimumState ComputeNumerical(const MnFcn &, const MinimumState &, const MnUserTransformation &, unsigned int maxcalls) const;
 
-   /// internal function to compute the Hessian using an analytical computation or externally provided in the FCNGradientBase class
-   MinimumState ComputeAnalytical(const FCNGradientBase &, const MinimumState &, const MnUserTransformation &) const;
+   /// internal function to compute the Hessian using an analytical computation or externally provided in the FCNBase class
+   MinimumState ComputeAnalytical(const FCNBase &, const MinimumState &, const MnUserTransformation &) const;
 
    MnStrategy fStrategy;
 };

diff --git a/math/minuit2/inc/Minuit2/MnMigrad.h b/math/minuit2/inc/Minuit2/MnMigrad.h
@@ -70,48 +70,6 @@ class MnMigrad : public MnApplication {
    {
    }
 
-   // constructs from gradient FCN
-
-   /// construct from FCNGradientBase + std::vector for parameters and errors
-   MnMigrad(const FCNGradientBase &fcn, std::span<const double> par, std::span<const double> err,
-            unsigned int stra = 1)
-      : MnApplication(fcn, MnUserParameterState(par, err), MnStrategy(stra)), fMinimizer(VariableMetricMinimizer())
-   {
-   }
-
-   /// construct from FCNGradientBase + std::vector for parameters and covariance
-   MnMigrad(const FCNGradientBase &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(VariableMetricMinimizer())
-   {
-   }
-
-   /// construct from FCNGradientBase + std::vector for parameters and MnUserCovariance
-   MnMigrad(const FCNGradientBase &fcn, std::span<const double> par, const MnUserCovariance &cov,
-            unsigned int stra = 1)
-      : MnApplication(fcn, MnUserParameterState(par, cov), MnStrategy(stra)), fMinimizer(VariableMetricMinimizer())
-   {
-   }
-
-   /// construct from FCNGradientBase + MnUserParameters
-   MnMigrad(const FCNGradientBase &fcn, const MnUserParameters &par, unsigned int stra = 1)
-      : MnApplication(fcn, MnUserParameterState(par), MnStrategy(stra)), fMinimizer(VariableMetricMinimizer())
-   {
-   }
-
-   /// construct from FCNGradientBase + MnUserParameters + MnUserCovariance
-   MnMigrad(const FCNGradientBase &fcn, const MnUserParameters &par, const MnUserCovariance &cov, unsigned int stra = 1)
-      : MnApplication(fcn, MnUserParameterState(par, cov), MnStrategy(stra)), fMinimizer(VariableMetricMinimizer())
-   {
-   }
-
-   /// construct from FCNGradientBase + MnUserParameterState + MnStrategy
-   MnMigrad(const FCNGradientBase &fcn, const MnUserParameterState &par, const MnStrategy &str)
-      : MnApplication(fcn, MnUserParameterState(par), str), fMinimizer(VariableMetricMinimizer())
-   {
-   }
-
    ~MnMigrad() override {}
 
    /// Copy constructor, copy shares the reference to the same FCNBase in MnApplication

diff --git a/math/minuit2/inc/Minuit2/MnMinimize.h b/math/minuit2/inc/Minuit2/MnMinimize.h
@@ -67,48 +67,6 @@ class MnMinimize : public MnApplication {
    {
    }
 
-   // interfaces using FCNGradientBase
-
-   /// construct from FCNGradientBase + std::vector for parameters and errors
-   MnMinimize(const FCNGradientBase &fcn, std::span<const double> par, std::span<const double> err,
-              unsigned int stra = 1)
-      : MnApplication(fcn, MnUserParameterState(par, err), MnStrategy(stra)), fMinimizer(CombinedMinimizer())
-   {
-   }
-
-   /// construct from FCNGradientBase + std::vector for parameters and covariance
-   MnMinimize(const FCNGradientBase &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(CombinedMinimizer())
-   {
-   }
-
-   /// construct from FCNGradientBase + std::vector for parameters and MnUserCovariance
-   MnMinimize(const FCNGradientBase &fcn, std::span<const double> par, const MnUserCovariance &cov,
-              unsigned int stra = 1)
-      : MnApplication(fcn, MnUserParameterState(par, cov), MnStrategy(stra)), fMinimizer(CombinedMinimizer())
-   {
-   }
-
-   /// construct from FCNGradientBase + MnUserParameters
-   MnMinimize(const FCNGradientBase &fcn, const MnUserParameters &par, unsigned int stra = 1)
-      : MnApplication(fcn, MnUserParameterState(par), MnStrategy(stra)), fMinimizer(CombinedMinimizer())
-   {
-   }
-
-   /// construct from FCNGradientBase + MnUserParameters + MnUserCovariance
-   MnMinimize(const FCNGradientBase &fcn, const MnUserParameters &par, const MnUserCovariance &cov,
-              unsigned int stra = 1)
-      : MnApplication(fcn, MnUserParameterState(par, cov), MnStrategy(stra)), fMinimizer(CombinedMinimizer())
-   {
-   }
-
-   /// construct from FCNGradientBase + MnUserParameterState + MnStrategy
-   MnMinimize(const FCNGradientBase &fcn, const MnUserParameterState &par, const MnStrategy &str)
-      : MnApplication(fcn, MnUserParameterState(par), str), fMinimizer(CombinedMinimizer())
-   {
-   }
-
    MnMinimize(const MnMinimize &migr)
       : MnApplication(migr.Fcnbase(), migr.State(), migr.Strategy(), migr.NumOfCalls()), fMinimizer(migr.fMinimizer)
    {