search

tBuLi / symfit

Symbolic Fitting; fitting as it should be.
http://symfit.readthedocs.org
MIT License
233 stars 17 forks source link

ConstraintNumericalLeastSquares without bounds #121

Closed Jhsmit closed 7 years ago

Jhsmit commented 7 years ago

If you fit using ConstraintNumbericalLeastSquares without specifying bounds:

xdata, ydata = np.genfromtxt('fit_data.txt').T

x = Variable(name='x')
A = Parameter(name='A', value=200)
sig = Parameter(name='sig', value=50)
x0 = Parameter(name='x0', value=500)
# Gaussian distrubution
model = A*exp(-((x - x0)**2/(2 * sig**2)))

fit = ConstrainedNumericalLeastSquares(model, xdata, ydata)
fit_result = fit.execute()
print(fit_result)
print(model)

You get the following error message:

C:\Miniconda3\envs\py3_new\lib\site-packages\scipy\optimize\slsqp.py:341: RuntimeWarning: invalid value encountered in greater
  bnderr = bnds[:, 0] > bnds[:, 1]C:\Miniconda3\envs\py3_new\lib\site-packages\scipy\optimize\slsqp.py:341: RuntimeWarning: invalid value encountered in greater
  bnderr = bnds[:, 0] > bnds[:, 1]

This is because self.models.bounds is in this case [(None, None), (None, None), (None, None)] which does not evaluate to None when checked by scipy slsqp (line 330). Its doesnt affect the fit in this case, but I can image it can cause problems.

tBuLi commented 7 years ago

Thanks for pointing this out. However, I think that apart from an annoying warning message this is harmless.

In your example where self.models.bounds returns [(None, None), (None, None), (None, None)], it could have been replaced by just None. But in the case where you set even just one, it has to be e.g. [(0.0, None), (None, None), (None, None)].

Every line in the code of scipy slsqp is fine with this, except the offending line 341. But the end result is that the None are always replaced by NaN before being passed to the Fortran code in both scenarios of the if ... else ....

I agree that this warning doesn't actually provide any useful information, but I'm not sure we should just catch it in symfit. Maybe this is something to fix in scipy (or there is a reason they didn't). Any ideas?

pckroon commented 7 years ago

I'd flag this as a wontfix. It looks to me as inconsistent input handling on scipy's side. If you want you (@tBuLi) can file an issue there.