Fix depwarns on julia 0.6 & Optim 0.7.5 #38
Conversation
test/curve_fit.jl
Outdated
| @@ -1,6 +1,6 @@ | |||
| let | |||
| # fitting noisy data to an exponential model | |||
| model(x, p) = p[1]*exp(-x.*p[2]) | |||
| model(x, p) = @compat p[1] .* exp.((-).(x) .* p[2]) | |||
There was a problem hiding this comment.
-x really doesn't work anymore??
There was a problem hiding this comment.
I think you've overdone it here... it should be sufficient to just vectorize the call to exp so it is:
p[1] * exp.(-x .* p[2])
There was a problem hiding this comment.
This just make the whole expression a single broadcast.
There was a problem hiding this comment.
Yes, but it does it at the expense of readability. We've made various choices in LsqFit to sacrifice a small amount of performance in the name of readability. In this case, I find (-).(x) to be extraordinarily obtuse. So, please... let's use the easier to read version.
There was a problem hiding this comment.
I'd always make the opposite sacrifice for a 2x reduction of allocation especially for non-public API but I guess this is only the test so I don't care. I changed it to the slower version and left TODO's for how to update it in the future.
|
Weren't dot calls in Compat 0.7.15? JuliaLang/Compat.jl@03fd3eb |
|
I usually use JuliaLang/Compat.jl@fbf0173 in order to not having to check if the case was supported correctly by 0.4 broadcast. |
|
do the tests fail with earlier versions? |
|
I don't have an old version so I don't know. It's a behavioral change though so I see 0.9.1 as the first version this is properly implemented. |
A Optim.jl PR is also ready and will be submitted after JuliaLang/Compat.jl#326 is tagged (Hopefully tomorrow).