Docs not very precise regarding **fit_kws for SCALAR_METHODS

[paulmueller]

I believe there is an issue in lmfit which has be raised before here #464, but in my opinion has been prematurely dismissed.

The docs state that the **fit_kws keyword arguments are supposed to be "Options to pass to the minimizer being used".

I think the documentation is not clear enough here. What is the minimizer method? A look in the code reveals that it is e.g. for "nelder" the scipy.optimize.minimize method. But that is not clear from the docs.

If I wanted to pass options to the minimizer method (e.g. "nelder"), then I have to wrap them in an options dictionary . However, here I can make out an additional issue.

Here, the options dictionary is created, but if the user specifies a custom options dict, it will override the defaults for 'maxiter'.

method = "Nelder-Mead"
fmin_kws = dict(method=method, options={'maxiter': 200})
kws = {'options': {"fatol": 1e-20}}
fmin_kws.update(kws)

print(fmin_kws)
# {'method': 'Nelder-Mead', 'options': {'fatol': 1e-20}}
# (maxiter is gone)

I believe the docs should be more elaborate (e.g. state that for SCALAR_METHODS, an "options" dictionary may be passed in addition to the tol etc. keyword arguments) and I believe the "options" dictionary should be updated recursively (not overridden) in Minimizer.scalar_minimize.

Thanks!

[newville]

@paulmueller

I believe there is an issue in lmfit which has be raised before here #464, but in my opinion, has been prematurely dismissed.

You are referring to an issue raised 3 1/2 years ago that was closed by the original poster on the same day it was raised. Some of us might view that more as "quickly resolved" than "prematurely dismissed".

The docs state that the **fit_kws keyword arguments are supposed to be "Options to pass to the minimizer being used".

I think the documentation is not clear enough here. What is the minimizer method?

The method specified by the user with the method argument.

A look in the code reveals that it is e.g. for "nelder" the scipy.optimize.minimize method. But that is not clear from the docs.

Is https://lmfit.github.io/lmfit-py/fitting.html#choosing-different-fitting-methods insufficient? Should the table there provide a link to the scipy.minimize doc?

Here, the options dictionary is created, but if the user specifies a custom options dict, it will override the defaults for 'maxiter'.

method = "Nelder-Mead"
fmin_kws = dict(method=method, options={'maxiter': 200})
kws = {'options': {"fatol": 1e-20}}
fmin_kws.update(kws)

print(fmin_kws)
# {'method': 'Nelder-Mead', 'options': {'fatol': 1e-20}}
# (maxiter is gone)

That seems sort of normal to me: you overwrote options. If you want to update the options, you could do that, no?

I believe the docs should be more elaborate (e.g. state that for SCALAR_METHODS, an "options" dictionary may be passed in addition to the tol etc. keyword arguments)

updates to the docs are always welcome.

and I believe the "options" dictionary should be updated recursively (not overridden) in Minimizer.scalar_minimize.

What if someone wants to completely overwrite it? That is: when should the user need to be explicit about setting these non-default options? I sort of think "always" is a defensible answer, but I would not say it is the only way to do it.

[paulmueller]

@newville Thanks for your reply.

Is https://lmfit.github.io/lmfit-py/fitting.html#choosing-different-fitting-methods insufficient? Should the table there provide a link to the scipy.minimize doc?

I actually think that the docs should point out that there are scalar minimizers in scipy that work differently than e.g. leastsq. For those scalar minimizers, the user should pass an options dictionary. Unfortunately, the scipy docs are also not very clear. From https://docs.scipy.org/doc/scipy/reference/optimize.minimize-neldermead.html, it is not immediately obvious that the "Options" listed in the doc string actually have to be in a separate dictionary. You easily overread that part there.

That seems sort of normal to me: you overwrote options. If you want to update the options, you could do that, no?

I would not expect to override any sensible defaults set by lmfit, if I only wanted to add my own options. In the current situation, if I wanted to set fatol, then I would "reset" the maxiter value.

and I believe the "options" dictionary should be updated recursively (not overridden) in Minimizer.scalar_minimize.

What if someone wants to completely overwrite it? That is: when should the user need to be explicit about setting these non-default options? I sort of think "always" is a defensible answer, but I would not say it is the only way to do it.

I would expect that the docs state that lmfit sets a sensible default value for "maxiter" (BTW why is that more sensible than the scipy default value?) in the options dict and what it is. And if you wanted to get the scipy default for "maxiter", then you should set it to None.

updates to the docs are always welcome.

I can make a PR once we agree on whether options should be updated recursively or not.

[newville]

@paulmueller

I actually think that the docs should point out that there are scalar minimizers in scipy that work differently than e.g. leastsq.

All the minimizers work differently from each other.

For those scalar minimizers, the user should pass an options dictionary.

The user can pass an options dictionary.

Unfortunately, the scipy docs are also not very clear. From https://docs.scipy.org/doc/scipy/reference/optimize.minimize-neldermead.html, it is not immediately obvious that the "Options" listed in the doc string actually have to be in a separate dictionary. You easily overread that part there.

What you did has nothing to do with lmfit or scipy at all. What you did is pure Python:

fmin_kws = dict(method=method, options={'maxiter': 200})
kws = {'options': {"fatol": 1e-20}}
fmin_kws.update(kws)

print(fmin_kws)
# {'method': 'Nelder-Mead', 'options': {'fatol': 1e-20}}

Yes, that is correct Python. You replaced the value of fmin_kws['options']. You could have done something like

fmin_kws['options'].update({'fatol': 1e-20})

or a few other variations of "add/change value of existing dict", but you did not. You erased the old dict that was held in fmin_kws['options'] and replaced it with another one. We cannot fix that.

[paulmueller]

@newville

First, I would like to point out that there already have been several misunderstandings in this thread. I did not mean to offend anyone. If I did, I'm sorry.

I just added this code example, because this is the way it is currently implemented in lmfit and I think this is wrong. In my opinion, the dictionary update should be recursive.

I know lmfit as a powerful library that makes things very easy for me. But in this particular part, lmfit has room for improvement. The suggestions I am making are:

• Don't replace the "options" dictionary when the user specifies it, but update it.
• Update the docs explicitly stating that for scalar minimizers, an "options" dictionary may be specified (and optionally reference the scipy docs)

[newville]

@paulmueller

Honestly, I do not know what you mean by the word 'recursive'.

The only example of code that you provided is pure Python, showing how dictionaries work. It has nothing to do with lmfit or scipy.optimize.

Your example is perfectly reasonable code and doing exactly what you said for Python to do. It may not say what you intended, but there really is not a way for us to know that or fix that. I don't see how it reveals any problem or really any information at all about lmfit or scipy.optimize.

[paulmueller]

@newville

Please take a look at https://github.com/lmfit/lmfit-py/blob/1.0.3/lmfit/minimizer.py#L955-L962.

Instead of (line 962)

fmin_kws.update(kws)

I would propose (sufficient to recurse one hierarchy down in the nested dictionary here, I believe):

for key in kws:
    if isinstance(kws[key], dict):
        subdict = fmin_kws.setdefault(key, {})
        subdict.update(kws[key])
    else:
        fmin_kws[key] = kws[key]

Of course, since probably only the "options" dictionary is affected anyway, one could also do

for key in kws:
    if key == "options":
        # Do not override "maxiter" in line 955
        fmin_kws["options"].update(kws["options"])
    else:
        fmin_kws[key] = kws[key]