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

MethodError for opt.initial_step #132

Closed
mzaffalon opened this issue Apr 15, 2019 · 4 comments · Fixed by #193
Closed

MethodError for opt.initial_step #132

mzaffalon opened this issue Apr 15, 2019 · 4 comments · Fixed by #193

Comments

@mzaffalon
Copy link
Contributor

mzaffalon commented Apr 15, 2019

No method is defined when invoking the initial_step property:

julia> using NLopt; opt = Opt(:LN_NELDERMEAD, 2);
julia> opt.initial_step
ERROR: MethodError: no method matching initial_step(::Opt)

Would

initial_step(o::Opt) = initial_step(o, zeros(Float64, ndims(o)))

fix it?

PS: I thought

initial_step(o::Opt) = initial_step(o, Vector{Float64}(undef, ndims(o)))

would work too, but the values I get make no sense.

@stevengj
Copy link
Collaborator

stevengj commented Apr 15, 2019

We could define initial_step(o::Opt) = x -> initial_step(o, x) so that you could do opt.initial_step(x), I guess, but that's maybe a little too OOP for Julia. Or just remove initial_step from getproperty … it was an oversight, probably, since I forgot that initial_step didn't work like that.

@mzaffalon
Copy link
Contributor Author

What is wrong with creating the temporary array in my proposal above?

By the way, I also see that you defined opt.inequality_constraint = ..., which sets the tolerance to 0. There are now two methods to define the constraints, using properties and the function call.

I find it a bit confusing having some methods being implemented as function calls only (e.g. ndims(opt)), some as get_property (e.g. opt.min_objective = ...) and some (e.g. inequality_constraints) exposing two different interfaces according to whether one passes the tolerance.

Yes, I know that I can call min_objective(opt, ...) as before.

@stevengj
Copy link
Collaborator

What is wrong with creating the temporary array in my proposal above?

Because the initial step in general depends on the initial guess vector, and there is no reason that initial guess vector should be the zero vector.

@mzaffalon
Copy link
Contributor Author

mzaffalon commented Apr 16, 2019

there is no reason that initial guess vector should be the zero vector.

I forgot about that.

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

Successfully merging a pull request may close this issue.

2 participants