Optimization based solvers #350
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Benchmark ResultsClick to check benchmark results
|
This comment was marked as outdated.
This comment was marked as outdated.
|
I think this PR is ready to go now. The optimal control interface still needs some more work considering the complex whole-time interval inequality(similar to |
| if prob isa NonlinearLeastSquaresProblem | ||
| return __FastShortcutBVPCompatibleNLLSPolyalg(eltype(prob.u0)) | ||
| else | ||
| return __FastShortcutBVPCompatibleNonlinearPolyalg(eltype(prob.u0)) | ||
| end | ||
| end | ||
|
|
||
| # Some optimization algorithms (solvers from interfacing packages) don't support the __solve(prob) interface |
There was a problem hiding this comment.
Interesting, we should handle this upstream, which ones?
| retcode = ifelse(SciMLBase.successful_retcode(nlsol), odesol.retcode, nlsol.retcode) | ||
| return SciMLBase.solution_new_original_retcode(odesol, nlsol, retcode, nlsol.resid) | ||
| end | ||
| function __build_solution(prob::AbstractBVProblem, odesol, optsol::SciMLBase.OptimizationSolution) | ||
| retcode = ifelse(SciMLBase.successful_retcode(optsol), odesol.retcode, optsol.retcode) | ||
| return SciMLBase.solution_new_original_retcode(odesol, optsol, retcode, zeros(length(first(odesol)))) # Need a patch in SciMLBase |
Comment on lines
+632
to
+638
| zeros(T, N*M) | ||
| else | ||
| lcons_length = length(prob.lcons) | ||
| vcat(prob.lcons, zeros(T, N*M - lcons_length)) | ||
| end | ||
| ucons = if isnothing(prob.ucons) | ||
| zeros(T, N*M) |
There was a problem hiding this comment.
Using zero is likely not good? That will mess with the stability of the solver quite a bit. If inequality constraints are given then the lcons / ucons should be required from the user.
| jac_prototype = jac_prototype) | ||
| return __internal_nlsolve_problem(prob, resid_prototype, y, nlf, y, p) | ||
| else | ||
| optf = OptimizationFunction{true}(__default_cost(prob.f), AutoFiniteDiff(), # Need to investigate the ForwardDiff dual problem |
Comment on lines
+694
to
+699
| jac_prototype = jac_prototype) | ||
| return __internal_nlsolve_problem(prob, resid_prototype, y, nlf, y, p) | ||
| else | ||
| optf = OptimizationFunction{true}( | ||
| __default_cost(prob.f), get_dense_ad(alg.jac_alg.diffmode), | ||
| cons = loss, cons_j = jac, cons_jac_prototype = jac_prototype) |
There was a problem hiding this comment.
It's not easy to see from this, but when doing multiple shooting with this the codegen should be a little different from the nonlinear solve. With multiple shooting you have the endpoint conditions, and you put those into the nonlinear equation to be satisfied. In the optimization, they should be moved to the equality constraints.
ChrisRackauckas
approved these changes
Sep 7, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #336
Some TODOs: