lmfit / lmfit-py

Non-Linear Least Squares Minimization, with flexible Parameter settings, based on scipy.optimize, and with many additional classes and methods for curve fitting.
https://lmfit.github.io/lmfit-py/
Other
1.07k stars 275 forks source link

Minor fixes to make scalar minimization more usable #80

Closed sdh4 closed 10 years ago

sdh4 commented 10 years ago

I've needed to make three minor fixes to lmfit, listed below, to get scalar minimization with the conjugate gradient (cg) algorithm working. All changes are to lmfit/minimizer.py:

1. Rationale: The way scalar_minimize is declared and used the 'tol=None' parameter seems to override, and yet it cannot be set explicitly through minimize(). With this change and the first part of #2, below, a tol= parameter on the minimize() command line is properly passed through to the scipy minimize code. (perhaps hess= should be removed also)

Patch:

@@ -343,7 +343,7 @@ or set  leastsq_kws['maxfev']  to increase this maximum."""
         self.unprepare_fit()
         return

-    def scalar_minimize(self, method='Nelder-Mead', hess=None, tol=None, **kws):
+    def scalar_minimize(self, method='Nelder-Mead', hess=None, **kws):
         """use one of the scaler minimization methods from scipy.
         Available methods include:
           Nelder-Mead

2. Rationale: The first line of this change completes fixing the tolerance parameter. The remainder of the change allows passing a jacobian to minimizers (such as cg) that can make use of one. Before this fix, while jac can be supplied, it does not get user arguments, and its parameters are not scaled according to the algorithm used to make variables bounded.

Patch:

@@ -376,10 +376,20 @@ or set  leastsq_kws['maxfev']  to increase this maximum."""
         if method not in ('L-BFGS-B', 'TNC', 'SLSQP'):
             opts['maxfev'] = maxfev

-        fmin_kws = dict(method=method, tol=tol, hess=hess, options=opts)
+        fmin_kws = dict(method=method, hess=hess, options=opts)
         fmin_kws.update(self.kws)
         fmin_kws.update(kws)

+        if 'Dfun' in fmin_kws and fmin_kws['Dfun'] is not None and not isinstance(fmin_kws['Dfun'],bool):
+            # Provided an explicit derivative (jacobian) function
+            self.jacfcn = fmin_kws['Dfun']
+
+            # scipy.minimize uses 'jac' to name the jacobian parameter, not Dfun
+            del fmin_kws['Dfun'] 
+            fmin_kws['jac'] = self.__jacobian
+
+
+
         ret = scipy_minimize(self.penalty, self.vars, **fmin_kws)
         xout = ret.x
         self.message = ret.message

3: Rationale: The 'CG' in the _scalar_methods dictionary contains an extraneous space, which prevents it from being used.

Patch:

@@ -528,7 +538,7 @@ def minimize(fcn, params, method='leastsq', args=None, kws=None,
                        iter_cb=iter_cb, scale_covar=scale_covar, **fit_kws)

     _scalar_methods = {'nelder': 'Nelder-Mead',     'powell': 'Powell',
-                       'cg': 'CG ',                 'bfgs': 'BFGS',
+                       'cg': 'CG',                  'bfgs': 'BFGS',
                        'newton': 'Newton-CG',       'anneal': 'Anneal',
                        'lbfgs': 'L-BFGS-B',         'l-bfgs': 'L-BFGS-B',
                        'tnc': 'TNC',                'cobyla': 'COBYLA',
newville commented 10 years ago

Thanks!. I will fix your points 1 and 3 ASAP, and will look into the best way to deal with your point 2 and the various names for Dfun/jac.... Is testing whether Dfun is a bool needed? That is, is passing Dfun=True/False likely?

newville commented 10 years ago

Done, mostly. For points 1 and 2, minimize() can now take 'tol', 'jac', 'hess' keyword arguments that will be used by scalar_minimize(), or these can be given directly to scalar_minimize(). For point 2, if 'Dfun' was used (and 'jac' not used), 'Dfun' will be used as 'jac'. It's unfortunate that both variations are used. This is all done with **keywords, which is generally poor for auto-documentation, but I think necessary here because there are so many variations.

I didn't include the check for 'jac=True', signifying that the objective function returns residual AND Jacobian because that is really, really ugly. The objective function should return the residual so that it can be used consistently with all methods.

sdh4 commented 10 years ago

I agree about jac==True. I don't see any reason to support that.

... In order for the jacobian function to work properly (provide extra args, scale inputs through bounding transformation), something like:

self.jacfcn = fmin_kws['jac'] fmin_kws['jac'] = self.__jacobian

is still needed.

Thanks!

newville commented 10 years ago

On Mon, Jan 6, 2014 at 9:59 AM, sdh4 notifications@github.com wrote:

I agree about jac==True. I don't see any reason to support that.

... In order for the jacobian function to work properly (provide extra args, scale inputs through bounding transformation), something like:

self.jacfcn = fmin_kws['jac'] fmin_kws['jac'] = self.__jacobian

is still needed.

Thanks!

— Reply to this email directly or view it on GitHubhttps://github.com/lmfit/lmfit-py/issues/80#issuecomment-31659221 .

Ahh, right.... fixed.

--Matt

newville commented 10 years ago

I believe this has been fully addressed