Re #4158. Changed BFGS_Minimizer.

Code/Mantid/Framework/CurveFitting/CMakeLists.txt

@@ -1,6 +1,6 @@
 set ( SRC_FILES
   src/Abragam.cpp
-#	src/BFGS_Minimizer.cpp
+	src/BFGS_Minimizer.cpp
 	src/BackToBackExponential.cpp
 	src/BackgroundFunction.cpp
 #	src/BivariateNormal.cpp
@@ -59,7 +59,7 @@ set ( SRC_UNITY_IGNORE_FILES src/Fit1D.cpp src/GSLFunctions.cpp )
 
 set ( INC_FILES
   inc/MantidCurveFitting/Abragam.h
-#	inc/MantidCurveFitting/BFGS_Minimizer.h
+	inc/MantidCurveFitting/BFGS_Minimizer.h
 	inc/MantidCurveFitting/BackToBackExponential.h
 	inc/MantidCurveFitting/BackgroundFunction.h
 #	inc/MantidCurveFitting/BivariateNormal.h
@@ -128,6 +128,7 @@ set ( TEST_FILES
 	test/StaticKuboToyabeTest.h
 	test/StretchExpMuonTest.h
 #	test/BackToBackExponentialTest.h
+	test/BFGSTest.h
 #	test/BivariateNormalTest.h
 #	test/BoundaryConstraintTest.h
 #	test/ChebyshevTest.h

Code/Mantid/Framework/CurveFitting/inc/MantidCurveFitting/BFGS_Minimizer.h

@@ -5,10 +5,7 @@
 // Includes
 //----------------------------------------------------------------------
 #include "MantidCurveFitting/DllConfig.h"
-#include "MantidCurveFitting/IFuncMinimizer.h"
-#include "MantidCurveFitting/GSLFunctions.h"
-#include <gsl/gsl_multimin.h>
-#include <gsl/gsl_multifit_nlin.h>
+#include "MantidCurveFitting/DerivMinimizer.h"
 
 namespace Mantid
 {
@@ -40,43 +37,21 @@ namespace CurveFitting
     File change history is stored at: <https://svn.mantidproject.org/mantid/trunk/Code/Mantid>.
     Code Documentation is available at: <http://doxygen.mantidproject.org>
 */
-class DLLExport BFGS_Minimizer : public IFuncMinimizer
+class DLLExport BFGS_Minimizer : public DerivMinimizer
 {
 public:
-  /// constructor and destructor
-  ~BFGS_Minimizer();
-  BFGS_Minimizer(): m_name("BFGS") {}
+  /// Constructor.
+  BFGS_Minimizer():DerivMinimizer()  {}
+  /// Name of the minimizer.
+  std::string name() const {return "BFGS_Minimizer";}
 
-  /// Overloading base class methods
-  std::string name()const;
-  int iterate();
-  int hasConverged();
-  double costFunctionVal();
-  void calCovarianceMatrix(double epsrel, gsl_matrix * covar);
-  void initialize(double* X, const double* Y, double *sqrtWeight, const int& nData, const int& nParam, 
-    gsl_vector* startGuess, API::IFitFunction* function, const std::string& costFunction);
-  void initialize(API::IFitFunction* function, const std::string& costFunction);
+protected:
 
-private:
-  /// name of this minimizer
-  const std::string m_name;
+  /// Return a concrete type to initialize m_gslSolver with
+  virtual const gsl_multimin_fdfminimizer_type* getGSLMinimizerType();
 
-  /// pointer to the GSL solver doing the work
-  gsl_multimin_fdfminimizer *m_gslSolver;
-
-  /// passed information about the derivative etc of fitting function
-  /// rather than the derivative etc of cost function
-  /// used for calculating covariance matrix
-  gsl_multifit_function_fdf m_gslLeastSquaresContainer;
-
-  /// GSL data container
-  GSL_FitData *m_data;
-
-  /// GSL container
-  gsl_multimin_function_fdf m_gslMultiminContainer;
-
-        /// Static reference to the logger class
-        static Kernel::Logger& g_log;
+  /// Static reference to the logger class
+  static Kernel::Logger& g_log;
 };
 
 

Code/Mantid/Framework/CurveFitting/inc/MantidCurveFitting/DerivMinimizer.h

@@ -61,7 +61,7 @@ class DLLExport DerivMinimizer : public IFuncMinimizer
 protected:
 
   /// Return a concrete type to initialize m_gslSolver with
-  virtual gsl_multimin_fdfminimizer_type* getGSLMinimizerType() = 0;
+  virtual const gsl_multimin_fdfminimizer_type* getGSLMinimizerType() = 0;
 
   /// Function to minimize.
   API::ICostFunction_sptr m_costFunction;

Code/Mantid/Framework/CurveFitting/src/BFGS_Minimizer.cpp

@@ -16,102 +16,11 @@ DECLARE_FUNCMINIMIZER(BFGS_Minimizer,BFGS)
 // Get a reference to the logger
 Kernel::Logger& BFGS_Minimizer::g_log = Kernel::Logger::get("BFGS_Minimizer");
 
-void BFGS_Minimizer::initialize(double* X, const double* Y, 
-                                              double *sqrtWeight, const int& nData, 
-                                              const int& nParam, gsl_vector* startGuess, API::IFitFunction* function, 
-                                              const std::string& costFunction) 
-{
-  UNUSED_ARG(X);
-  UNUSED_ARG(Y);
-  UNUSED_ARG(sqrtWeight);
-  // set-up GSL container to be used with GSL simplex algorithm
-  m_data = new GSL_FitData(function,API::CostFunctionFactory::Instance().createUnwrapped(costFunction));
-
-  const gsl_multimin_fdfminimizer_type *T = gsl_multimin_fdfminimizer_vector_bfgs2;
-
-  // setup GSL container
-  m_gslMultiminContainer.n = nParam;  
-  m_gslMultiminContainer.f = &gsl_costFunction;
-  m_gslMultiminContainer.df = &gsl_costFunction_df;
-  m_gslMultiminContainer.fdf = &gsl_costFunction_fdf;
-  m_gslMultiminContainer.params = m_data;
-
-  // setup minimizer
-  m_gslSolver = gsl_multimin_fdfminimizer_alloc(T, nParam);
-  gsl_multimin_fdfminimizer_set(m_gslSolver, &m_gslMultiminContainer, startGuess, 0.01, 0.01);
-
-  // for covariance matrix
-  m_gslLeastSquaresContainer.f = &gsl_f;
-  m_gslLeastSquaresContainer.df = &gsl_df;
-  m_gslLeastSquaresContainer.fdf = &gsl_fdf;
-  m_gslLeastSquaresContainer.n = nData;
-  m_gslLeastSquaresContainer.p = nParam;
-  m_gslLeastSquaresContainer.params = m_data;
-}
-
-void BFGS_Minimizer::initialize(API::IFitFunction* function, const std::string& costFunction) 
-{
-  // set-up GSL container to be used with GSL simplex algorithm
-  m_data = new GSL_FitData(function,API::CostFunctionFactory::Instance().createUnwrapped(costFunction));
-
-  const gsl_multimin_fdfminimizer_type *T = gsl_multimin_fdfminimizer_vector_bfgs2;
-
-  // setup GSL container
-  m_gslMultiminContainer.n = function->nActive();  
-  m_gslMultiminContainer.f = &gsl_costFunction;
-  m_gslMultiminContainer.df = &gsl_costFunction_df;
-  m_gslMultiminContainer.fdf = &gsl_costFunction_fdf;
-  m_gslMultiminContainer.params = m_data;
-
-  // setup minimizer
-  m_gslSolver = gsl_multimin_fdfminimizer_alloc(T, function->nActive());
-  gsl_multimin_fdfminimizer_set(m_gslSolver, &m_gslMultiminContainer, m_data->initFuncParams, 0.01, 0.01);
-
-  // for covariance matrix
-  m_gslLeastSquaresContainer.f = &gsl_f;
-  m_gslLeastSquaresContainer.df = &gsl_df;
-  m_gslLeastSquaresContainer.fdf = &gsl_fdf;
-  m_gslLeastSquaresContainer.n = function->dataSize();
-  m_gslLeastSquaresContainer.p = function->nActive();
-  m_gslLeastSquaresContainer.params = m_data;
-}
-
-BFGS_Minimizer::~BFGS_Minimizer()
-{
-  delete m_data;
-  gsl_multimin_fdfminimizer_free(m_gslSolver);
-}
-
-std::string BFGS_Minimizer::name()const
-{
-  return m_name;
-}
 
-int BFGS_Minimizer::iterate() 
+/// Return a concrete type to initialize m_gslSolver gsl_multimin_fdfminimizer_vector_bfgs2
+const gsl_multimin_fdfminimizer_type* BFGS_Minimizer::getGSLMinimizerType()
 {
-  return gsl_multimin_fdfminimizer_iterate(m_gslSolver);
-}
-
-int BFGS_Minimizer::hasConverged()
-{
-  return gsl_multimin_test_gradient(m_gslSolver->gradient, 1e-3);
-}
-
-double BFGS_Minimizer::costFunctionVal()
-{
-  return m_gslSolver->f;
-}
-
-void BFGS_Minimizer::calCovarianceMatrix(double epsrel, gsl_matrix * covar)
-{
-  gsl_matrix * holdCalculatedJacobian;
-  holdCalculatedJacobian =  gsl_matrix_alloc (m_gslLeastSquaresContainer.n, m_gslLeastSquaresContainer.p);
-
-  int dummy = m_gslLeastSquaresContainer.df(m_gslSolver->x, m_gslLeastSquaresContainer.params, holdCalculatedJacobian);
-  (void) dummy;
-  gsl_multifit_covar (holdCalculatedJacobian, epsrel, covar);
-
-  gsl_matrix_free (holdCalculatedJacobian);
+  return gsl_multimin_fdfminimizer_vector_bfgs2;
 }
 
 } // namespace CurveFitting

Code/Mantid/Framework/CurveFitting/src/DerivMinimizer.cpp

@@ -75,7 +75,7 @@ void DerivMinimizer::initialize(API::ICostFunction_sptr function)
   m_gslMultiminContainer.fdf = &fundfun;
   m_gslMultiminContainer.params = this;
 
-  gsl_multimin_fdfminimizer *s = gsl_multimin_fdfminimizer_alloc( getGSLMinimizerType(), m_gslMultiminContainer.n );
+  m_gslSolver = gsl_multimin_fdfminimizer_alloc( getGSLMinimizerType(), m_gslMultiminContainer.n );
 
   size_t nParams = m_costFunction->nParams();
   // Starting point 
@@ -98,9 +98,18 @@ bool DerivMinimizer::iterate()
     throw std::runtime_error("Minimizer " + this->name() + " was not initialized.");
   }
   int status = gsl_multimin_fdfminimizer_iterate(m_gslSolver);
-  if (status) return false;
+  if (status)
+  {
+    m_errorString = gsl_strerror(status);
+    return false;
+  }
   status = gsl_multimin_test_gradient (m_gslSolver->gradient, 1e-3); //! <---------------
-  return status == GSL_CONTINUE;
+  if (status != GSL_CONTINUE)
+  {
+    m_errorString = gsl_strerror(status);
+    return false;
+  }
+  return true;
 }
 
 bool DerivMinimizer::minimize() 

Code/Mantid/Framework/CurveFitting/test/BFGSTest.h

@@ -0,0 +1,85 @@
+#ifndef CURVEFITTING_BFGSTEST_H_
+#define CURVEFITTING_BFGSTEST_H_
+
+#include <cxxtest/TestSuite.h>
+
+#include "MantidCurveFitting/BFGS_Minimizer.h"
+#include "MantidAPI/ICostFunction.h"
+
+#include <sstream>
+
+using namespace Mantid;
+using namespace Mantid::CurveFitting;
+using namespace Mantid::API;
+
+class BFGSTestCostFunction: public ICostFunction 
+{
+  double a,b;
+public:
+  BFGSTestCostFunction():a(1),b(1){}
+  virtual std::string name() const {return "BFGSTestCostFunction";}
+  virtual double getParameter(size_t i)const
+  {
+    if (i == 0) return a;
+    return b;
+  }
+  virtual void setParameter(size_t i, const double& value)
+  {
+    if (i == 0)
+    {
+      a = value;
+    }
+    else
+    {
+      b = value;
+    }
+  }
+  virtual size_t nParams()const {return 2;}
+  virtual double val() const 
+  {
+    double x = a - 1.1;
+    double y = b - 2.2;
+    return 3.1 + x * x + y * y;
+  }
+  virtual void deriv(std::vector<double>& der) const 
+  {
+    if (der.size() != 2)
+    {
+      der.resize(2);
+    }
+    double x = a - 1.1;
+    double y = b - 2.2;
+    der[0] = 2 * x;
+    der[1] = 2 * y;
+  }
+  virtual double valAndDeriv(std::vector<double>& der) const 
+  {
+    deriv(der);
+    return val();
+  }
+};
+
+
+class BFGSTest : public CxxTest::TestSuite
+{
+public:
+  // This pair of boilerplate methods prevent the suite being created statically
+  // This means the constructor isn't called when running other tests
+  static BFGSTest *createSuite() { return new BFGSTest(); }
+  static void destroySuite( BFGSTest *suite ) { delete suite; }
+
+  void testMinimize()
+  {
+    ICostFunction_sptr fun(new BFGSTestCostFunction);
+    BFGS_Minimizer s;
+    s.initialize(fun);
+    TS_ASSERT(s.minimize());
+    TS_ASSERT_DELTA(fun->val(),3.1,1e-10);
+    TS_ASSERT_DELTA(fun->getParameter(0),1.1,1e-10);
+    TS_ASSERT_DELTA(fun->getParameter(1),2.2,1e-10);
+    TS_ASSERT_EQUALS(s.getError(),"success");
+  }
+
+};
+
+#endif /*CURVEFITTING_BFGSTEST_H_*/

Code/Mantid/Framework/CurveFitting/test/LeastSquaresTest.h

@@ -5,7 +5,9 @@
 
 #include "MantidCurveFitting/CostFuncLeastSquares.h"
 #include "MantidCurveFitting/SimplexMinimizer.h"
+#include "MantidCurveFitting/BFGS_Minimizer.h"
 #include "MantidCurveFitting/UserFunction.h"
+#include "MantidCurveFitting/ExpDecay.h"
 #include "MantidAPI/FunctionDomain1D.h"
 #include "MantidAPI/FunctionValues.h"
 
@@ -23,7 +25,7 @@ class LeastSquaresTest : public CxxTest::TestSuite
   static LeastSquaresTest *createSuite() { return new LeastSquaresTest(); }
   static void destroySuite( LeastSquaresTest *suite ) { delete suite; }
 
-  void testSimplex()
+  void test_With_Simplex()
   {
     std::vector<double> x(10),y(10);
     for(size_t i = 0; i < x.size(); ++i)
@@ -53,6 +55,35 @@ class LeastSquaresTest : public CxxTest::TestSuite
     TS_ASSERT_EQUALS(s.getError(),"success");
   }
 
+  void test_With_BFGS()
+  {
+    std::vector<double> x(10),y(10);
+    for(size_t i = 0; i < x.size(); ++i)
+    {
+      x[i] = 0.1 * i;
+      y[i] =  9.9 * exp( -(x[i])/0.5 );
+    }
+    API::FunctionDomain1D_sptr domain(new API::FunctionDomain1D(x));
+    API::FunctionValues_sptr values(new API::FunctionValues(*domain));
+    values->setFitData(y);
+    values->setFitWeights(1.0);
+
+    API::IFunction_sptr fun(new ExpDecay);
+    fun->setParameter("Height",1.);
+    fun->setParameter("Lifetime",1.);
+
+    boost::shared_ptr<CostFuncLeastSquares> costFun(new CostFuncLeastSquares);
+    costFun->setFittingFunction(fun,domain,values);
+
+    BFGS_Minimizer s;
+    s.initialize(costFun);
+    TS_ASSERT(s.minimize());
+    TS_ASSERT_DELTA(costFun->val(),0.0,1e-10);
+    TS_ASSERT_DELTA(fun->getParameter("Height"),3.3,1e-10);
+    TS_ASSERT_DELTA(fun->getParameter("Lifetime"),4.4,1e-10);
+    TS_ASSERT_EQUALS(s.getError(),"success");
+  }
+
 };
 
 #endif /*CURVEFITTING_LEASTSQUARESTEST_H_*/