Refactor code to make it Optimization.jl compatible #196
Conversation
src/build_loss_objective.jl
Outdated
| gcfg = ForwardDiff.GradientConfig(cost_function, autodiff_prototype, autodiff_chunk) | ||
| g! = (x, out) -> ForwardDiff.gradient!(out, cost_function, x, gcfg) | ||
| elseif flsa_gradient | ||
| if flsa_gradient |
There was a problem hiding this comment.
this is also an autodiff option
src/build_loss_objective.jl
Outdated
| end | ||
| cost_function(p) | ||
| else | ||
| cost_function2 = nothing |
There was a problem hiding this comment.
the cost_function2 can just in general be removed.
There was a problem hiding this comment.
Yeah even I thought that but wasn't sure if it would be too breaking
There was a problem hiding this comment.
Go for broke: This should get a major release with this change anyways. Just make these functions that return objective functions into functions that return an OptimizationProblem or something of the sort.
src/two_stage_method.jl
Outdated
| est_sol = preview_est_sol[i] | ||
| _du = f(est_sol, p, tpoints[i]) | ||
| cost += sum(abs2, vec(preview_est_deriv[i]) .- vec(_du)) |
Vaibhavdixit02
force-pushed
the
optimizationintegration
branch
2 times, most recently
from
d470e20
to
c84e0cb
Compare
Vaibhavdixit02
force-pushed
the
optimizationintegration
branch
2 times, most recently
from
06f7cf4
to
4b4e394
Compare
Vaibhavdixit02
force-pushed
the
optimizationintegration
branch
from
4b4e394
to
34a214c
Compare
Vaibhavdixit02
changed the title
| LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" | ||
| LsqFit = "2fda8390-95c7-5789-9bda-21331edee243" | ||
| PenaltyFunctions = "06bb1623-fdd5-5ca2-a01c-88eae3ea319e" | ||
| PreallocationTools = "d236fae5-4411-538c-8e31-a6e3d9e00b46" | ||
| RecursiveArrayTools = "731186ca-8d62-57ce-b412-fbd966d074cd" | ||
| SciMLBase = "0bca4576-84f4-4d90-8ffe-ffa030f20462" | ||
| SciMLSensitivity = "1ed8b502-d754-442c-8d5d-10ac956f44a1" |
There was a problem hiding this comment.
it might be good to keep a SciMLSensitivity because users just get a weird error telling them to add it if they use AutoZygote, and I think it won't be too uncommon to do that.
That said, a better solution may be to make SciMLSensitivity be a weak dependency of DiffEqBase that is added and used whenever Zygote, ReverseDiff, or Tracker are used. @oscardssmith is that possible?
| opt = Opt(:LN_BOBYQA, 3) | ||
| lower_bounds!(opt, [9.0, 20.0, 2.0]) | ||
| upper_bounds!(opt, [11.0, 30.0, 3.0]) | ||
| min_objective!(opt, obj_short.cost_function2) | ||
| xtol_rel!(opt, 1e-12) | ||
| maxeval!(opt, 10000) | ||
| @time (minf, minx, ret) = NLopt.optimize(opt, [9.0, 20.0, 2.0]) |
|
Let's merge but not tag until docs |
No description provided.