-
Notifications
You must be signed in to change notification settings - Fork 6
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
Added implicit midpoint discretization #228
Conversation
Could you give the possibility to provide a String or a Symbol? discretization = "midpoint"
# or
discretization = :midpoint You can simply add: function direct_solve(
ocp::OptimalControlModel,
description::Symbol...;
init = CTBase.__ocp_init(),
grid_size::Int = CTDirect.__grid_size(),
time_grid = CTDirect.__time_grid(),
discretization::Union{String, Symbol} = __discretization(),
kwargs...,
)
discretization = string(discretization)
...
end
# Idem for direct_transcription Note It would be nice to have this for all options given by a Symbol. |
Sure. By the way, yes, we should adopt the same rule for all arguments. Maybe always allow for String/Symbol ? On the other hand, just adding the string conversion as you wrote is not that hard. |
Note: the aforementioned plumbing caused some additional allocations and slower execution than the hardcoded trapeze. |
The conversion for all the options in function replace_symbols_by_strings(; kwargs...)
return replace(p -> p.second isa Symbol ? p.first => string(p.second) : p, kwargs)
end and the extension of equality: import Base: :(==)
Base.:(==)(a::String, b::Symbol) = a == string(b)
Base.:(==)(a::Symbol, b::String) = Base.:(==)(b, a) |
@PierreMartinon nice! yes, symbols are more or less the standard for julia options |
@PierreMartinon You can create if you want a file |
…nsufficient specialization in multiple dispatch
@PierreMartinon Je vois que tu es devenu fan des "Tag" :-) |
Actually I went back a bit from the separate tag, now the whole docp is parametrized by the discretization ie Merge and release probably next week, I'd like to add the Gauss Legendre 4th order scheme as well. |
@PierreMartinon seen that you have added quite a lot of things 👍🏽!
|
Such a big PR! |
@ocots @jbcaillau @joseph-gergaud Hi everyone,
We now have the implicit midpoint method in addition to the default explicit trapeze. The choice is made by setting the
discretization="midpoint"
argument in thedirect_solve
call.direct_transcription
takes the argument as well, I need to add some tests on this.Default trapeze method
Implicit midpoint method (note the larger NLP size due to the k_i variables)
Now that all the plumbing is in place, it should be quite easy to add new discretization schemes. I'll add a 2-stage gauss method to check, then back to allocation / performance aspects.