GaussianModel.guess() throws exception without optional `x` parameter

[kielpins]

First Time Issue Code

Yes, I read the instructions and I am sure this is a GitHub Issue.

Description

Running GaussianModel.guess() without the optional x parameter throws an AttributeError. I would expect the parameter guess to be returned without throwing an exception.

A Minimal, Complete, and Verifiable example

import numpy as np
from lmfit.models import GaussianModel

x = np.arange(100)
y = np.exp(-(x-50)**2/(2*10**2))

mod = GaussianModel()
pars = mod.guess(y)

Error message:

Traceback (most recent call last):
  File "C:/Users/davek/AppData/Roaming/JetBrains/PyCharm2021.2/scratches/scratch.py", line 8, in <module>
    pars = mod.guess(y)
  File "C:\Users\davek\base\lib\site-packages\lmfit\models.py", line 420, in guess
    return update_param_vals(pars, self.prefix, **kwargs)
  File "C:\Users\davek\base\lib\site-packages\lmfit\models.py", line 110, in update_param_vals
    pars.update_constraints()
AttributeError: 'tuple' object has no attribute 'update_constraints'

Version information

Python: 3.8.10 (tags/v3.8.10:3d8993a, May 3 2021, 11:48:03) [MSC v.1928 64 bit (AMD64)] lmfit: 1.0.2, scipy: 1.7.1, numpy: 1.21.2, asteval: 0.9.25, uncertainties: 3.1.6

[newville]

@kielpins

First Time Issue Code Yes, I read the instructions and I am sure this is a GitHub Issue.

The track record on first-time issues actually resulting in changes to the code is pretty low.

For sure, the Model.guess() functions are convenience functions, and may not necessarily work flawlessly. YMMV.

But also: A Gaussian model has 3 parameters: amplitude, center, and sigma. It turns out that the values for x are needed to guess the initial value for each of these. That is, I cannot tell what you expect to happen when you don't give GaussianModel.guess() values for x.

Is your idea just that a clearer message should be given?

[kielpins]

The track record on first-time issues actually resulting in changes to the code is pretty low.

Noted.

For sure, the Model.guess() functions are convenience functions, and may not necessarily work flawlessly. YMMV.

I wouldn't expect a documented function to throw an exception when an optional parameter is omitted.

But also: A Gaussian model has 3 parameters: amplitude, center, and sigma. It turns out that the values for x are needed to guess the initial value for each of these. That is, I cannot tell what you expect to happen when you don't give GaussianModel.guess() values for x.

Normally I would expect x to default to the array index. That's the usual convention for e.g. matplotlib.

[newville]

For sure, the Model.guess() functions are convenience functions, and may not necessarily work flawlessly. YMMV.

I wouldn't expect a documented function to throw an exception when an optional parameter is omitted.

Well, maybe the way to think about it is that the parameter is not actually optional. ;)

Normally I would expect x to default to the array index. That's the usual convention for e.g. matplotlib.

Hm, I'd stick with "In the face of ambiguity, refuse the temptation to guess."

So, if someone does

   x = np.linspace(8000, 8200, 101)
   y = np.exp(-(x-50)**2/(2*10**2))
   mod = GaussianModel()
   pars = mod.guess(y)
   result = mod.fit(y, pars, x=x)

the parameters will be generated OK, and the fit would just fail to find a good fit as the initial values are just ridiculously wrong. No, the initial parameter values are just always, always important and always depend on the actual values for x and y.

[reneeotten]

one possible option would be to add x to the guess function as a required argument, since I agree with @newville that this is the case anyway. Most built-in models already do def guess(self, data, x=None, <possible_other_argument>, **kwargs) and for the peak-like models x is explicitly used by the guess function.

I think we wouldn't have any backwards-incompatibility and/or breakage of scripts by end-user if we do the following (but we should test/think it through a bit if we would consider making such a change):

• change the guess function signature in model.py from def guess(self, data, **kws) to def guess(self, data, vals_independent_var, **kws) (of course the name vals_independent_var can be changed).
• adapt the guess methods for the built-in models in models.py to follow that

[newville]

@reneeotten @kielpins That would be totally fine with me. Giving a better warning of approximately "x is required for guess()" would be OK with me too.

I think those are about the same amount of work. For "required argument", the built-in language features give an exception. For the "we warn approach", each model subclass would need a line or two added for its independent variable(s), which are mostly just x. Either way, each XModel.guess() function has to be touched.