-
Notifications
You must be signed in to change notification settings - Fork 224
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
Fix input for (L)BFGS and PSO #431
Conversation
@inbounds state.x[i] = state.x[i] + state.dx[i] | ||
@simd for i = 1:n | ||
state.dx[i] .= state.alpha .* state.s[i] | ||
state.x[i] .= state.x[i] .+ state.dx[i] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will it not work to do state.x .= state.x .+ state.dx
? (Would dot-notation not work for arbitrary vector-types?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I got confused. I mean on the state.s
: I thought that's meant to be a vector, whilst state.x
may be a multidimensional array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a mistake! I had originally tried to change it to broadcast, but that didn't like mixing vectors and matrices.
I mean, state.x
is allowed to be multidimensional. If x
is passed as a matrix we want Optim.minimizer(res)
to be a matrix as well.
We could store s
as the same type as x
, and throw vec
's around state.s
whereever it shows up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would reshape on state.s work?
Alternatively reshape on Optim.minimizer(res)
, and then just have state.x
as a vector internally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, ofc you could just do minimizer = similar(initial_x); copy!(minimizer, state.x)
in the end, that's another possibility.
Codecov Report
@@ Coverage Diff @@
## master #431 +/- ##
==========================================
+ Coverage 90.46% 90.84% +0.38%
==========================================
Files 32 32
Lines 1562 1562
==========================================
+ Hits 1413 1419 +6
+ Misses 149 143 -6
Continue to review full report at Codecov.
|
test/multivariate/array.jl
Outdated
@test typeof(_minimizer) <: Array{Float64, 3} | ||
@test size(_minimizer) == (2,2,1) | ||
if !(m in (SimulatedAnnealing, ParticleSwarm)) | ||
println(typeof(m)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes yes, this is debugging text, will remove... :)
ref #399