-
Notifications
You must be signed in to change notification settings - Fork 227
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
Fixes 2/3 of problems with Nelder Mead related traces #942
Conversation
Codecov Report
@@ Coverage Diff @@
## master #942 +/- ##
==========================================
+ Coverage 83.60% 85.95% +2.34%
==========================================
Files 42 42
Lines 3019 3182 +163
==========================================
+ Hits 2524 2735 +211
+ Misses 495 447 -48
Continue to review full report at Codecov.
|
Thanks for looking into this. It looks fine. I would appreciate if you could add a test for the missing |
Sure, at least if you can slightly more specific on what you want. You just want me write a test that simply checks that the command works? In which of the many |
Yes, just checking that what you changed does what you intended. Here would be a good place https://github.com/JuliaNLSolvers/Optim.jl/blob/master/test/multivariate/solvers/zeroth_order/nelder_mead.jl. Just add a testset with title |
Okay, I'll get it done sometime "soon". EDIT: Accidentally closed it when I posted the last comment. Sorry. |
"Soon" became 6 months, but hey, who is counting? I see now that you wanted me to post a separate pull request, but hope this works. Note that, as mentioned in #941 that the stored trace is wrong, it replaces all simplex values with the last one. (That's why there are two Too see this: using Optim
f(x) = (1.0 - x[1])^2 + 100.0 * (x[2] - x[1]^2)^2
x0 = [0.0,0.0]
res = optimize(f, x0, NelderMead(),Optim.Options(store_trace=true,trace_simplex=true,extended_trace=true,iterations=2,show_trace=true))
display(res.trace) and compare the output of the simplexes (both y-values and x-simplexes) from the "running" trace and the stored res.trace. |
@pkofod Just pinging. (See comment above) |
Fixes the first two the issues raised in #941
I tried to fix the last one, but that was beyond my capabilities. If you give me a pointer I should be able to figure it out.