Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unnecessary passing of independent variable to model and curve_fit #149

Open
kagalenko-m-b opened this issue Feb 25, 2020 · 8 comments
Open

Comments

@kagalenko-m-b
Copy link

Optimization routine curve_fit() and function model() that should be supplied to it both require a user to pass the independent variable "xdata". Yet the optimization algorithm makes no use of it, other than passing to model().

This looks like unnecessary clutter. Why not omit this argument from both the model and optimization functions, supplying it through closure to the model (and Jacobian, if used)?

@pkofod
Copy link
Member

pkofod commented Mar 1, 2020

I don't understand what you mean. Can you clarify?

@kagalenko-m-b
Copy link
Author

kagalenko-m-b commented Mar 1, 2020

At present, I must define the model to pass to curve fit as:

function model(xdata, alpha) 
...
end

and then call curve_fit as

curve_fit(model, xdata, ydata, p0)

However, neither xdata nor ydata are needed by the optimization algorithm. The actual
call would be better as

xdata =[x1,x2,...]
ydata =[y1,y2,...]
f(alpha) = ydata - model(xdata, alpha)
curve_fit(f, p0)

The definition of f(alpha) captures xdata and ydata at the point of definition. The curve_fit() searches for the minimum norm solution.

@pkofod
Copy link
Member

pkofod commented Mar 1, 2020

Oh, okay, yes I see. I agree. I would guess this was originally made so because people have trouble unerstanding closures sometimes, but as I said, I agree. LsqFit will be revamped once Optim has been revamped, so I will keep this issue open to remember this point. Thanks.

@Magalame
Copy link
Contributor

I understand the point here, but it'd probably be nicer for most users to keep the current form, they should not need to know about optimization to use the package. Plus it's the habit that instituted scipy and matlab.

@kagalenko-m-b
Copy link
Author

I do not understand this objection at all, the simplified form requires exactly zero additional knowledge about optimization. Implementation of the the old interface in terms of the new one is literally one line:

curve_fit(model, xdata, ydata, p0) = curve_fit(x->ydata-model(xdata,x), p0)

@Magalame
Copy link
Contributor

Magalame commented Jun 23, 2020

I do not understand this objection at all

Let me break it down:

  • other packages use the current form
  • the form you are proposing does not make sense to a random user
  • the reverse is true: curve_fit(f, p0) = curve_fit(x -> f, [0], zeros(length(f(p0))), p0)

@kagalenko-m-b
Copy link
Author

kagalenko-m-b commented Jun 24, 2020

  • other packages use the current form

"Other packages", meaning MATLAB, use the verbose form because that language does not have closures. There's no reason to keep legacy crud in a better language.

(And btw, it's not really accurate to say that scipy and MATLAB use the same form. lsqnonlin() and scipy.optimize.least_squares() allow to pass extra arguments to the object function, but do not require them).

  • the form you are proposing does not make sense to a random user

That is an assertion not backed by any argument. Interface with fewer parameters is clearly easier to understand.

  • the reverse is true: curve_fit(f, p0) = curve_fit(x -> f, [0], zeros(length(f(p0))), p0)

The reverse is not really true, because in addition you need to add a whole lot of unnecessary argument passing within curve_fit().

@lamont-granquist
Copy link

FWIW I'm coming here from Matlab and looking for a lsqnonlin() replacement for multivariate root finding optimization and using this issue as the available documentation on how to get a curve_fit(f, p0) kind of interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants