-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Dev] Extensions #12
Comments
Another possibility is to write: using OptimalControl
using OptimalControlProblems
nlp = cart_pendulum(; model=:optimalcontrol) or using JuMP
using OptimalControlProblems
nlp = cart_pendulum(; model=:jump) with an error of the form: julia> using JuMP
julia> using OptimalControlProblems
julia> nlp = cart_pendulum(; model=:optimalcontrol)
ERROR: ExtensionError. Please make: julia> using OptimalControl Note In CTDirect.jl you have an example of such a case. See here to see how is defined the |
I would be in favor of not complexifying too much OptimalControlProblems.jl, and I don't think we should use package extensions at the moment. I think it's reasonable to assume both JuMP and OptimalControl as direct dependencies there, as they are both complementary to formulate OCP. Package extensions would make sense if we want to add more backends to the benchmark library. |
Regarding your second point, I think a more Julian way would be to use multiple dispatch. Something like: # template
function cart_pendulum end
abstract type AbstractOCPBackend end
struct JuMPBackend <: AbstractOCPBackend end
function cart_pendulum(::JuMPBackend)
# some code
end
struct OptimalControlBackend <: AbstractOCPBackend end
function cart_pendulum(::OptimalControlBackend)
# some code
end
The problem can be instantiated e.g. as nlp = cart_pendulum(JuMPBackend()) That way an error is automatically thrown if the backend is not defined. |
I agree with the fact that we should not complexify too much for the user and for someone who want to add models. I do not think that this is so complex to make an extension and for the user, it changes just the fact that he has to make an additional I would be in favor to make extensions because it is very nice to reduce as much as possible time for installation, precompilation, etc. One use case is the following: we will make some tests inside Besides, handling extensions errors permit to give a message more detailed. In Julia, the error messages are not easy to understand. @PierreMartinon Any thoughts? |
For the second point, I agree with you but I have hidden multiple dispatch. I like the idea that the user should not care about types. The function with the |
I see your point. In that case let's follow your idea.
I see. But is that compatible with package extensions? As far as I understand, you will have to define the structs function cart_pendulum(; model=:optimalcontrol)
if model == :optimalcontrol
cart_pendulum(OptimalControlBackend())
elseif model == :jump
cart_pendulum(JuMPBackend())
else
error("...")
end
end Is that the idea? If so I think this will lead to a lot of boilerplate code, and hinder the package's extensibility if we want to plug additional backends. |
For the second point you will have the following. In the main module: abstract type AbstractModelBackend end
struct OptimalControlBackend <: AbstractModelBackend end
struct JuMPBackend <: AbstractModelBackend end
function cart_pendulum(; model=:optimalcontrol)
if model == :optimalcontrol
cart_pendulum(OptimalControlBackend())
elseif model == :jump
cart_pendulum(JuMPBackend())
else
error("unknown model: ", model)
end
end
# function to be extended
function cart_pendulum(model_backend::AbstractModelBackend, args...; kwargs...)
if typeof(model_backend) == OptimalControlBackend
throw(ExtensionError(:OptimalControl)) # this provides an error which tells to make julia> using OptimalControl
elseif typeof(model_backend) == JuMPBackend
throw(ExtensionError(:JuMP))
end
end In the extension for OptimalControl models we would have: function cart_pendulum(model_backend::OptimalControlBackend; nh::Int=100)
...
end |
To make things shorter we can define in the main module: abstract type AbstractModelBackend end
struct OptimalControlBackend <: AbstractModelBackend end
struct JuMPBackend <: AbstractModelBackend end
backends = Dict(:optimalcontrol => OptimalControlBackend, :jump => JuMPBackend)
weakdeps = Dict(OptimalControlBackend => :OptimalControl, JuMPBackend => :JuMP)
function cart_pendulum(; model=:optimalcontrol)
model ∈ keys(backends) ? cart_pendulum(backends[model]()) : error("Unknown model: ", model)
end
function cart_pendulum(model_backend::T, args...; kwargs...) where {T <: AbstractModelBackend}
throw(ExtensionError(weakdeps[T])) # equivalent to error("Please make: julia> using ", weakdeps[T])
end and in the extension, it is as usual: function cart_pendulum(model_backend::OptimalControlBackend; nh::Int=100)
...
end Note The part in the main module may be automated from the files I guess. We only need the name of the problems. Note I am tired but we can also mutualize code for |
A version more generic :-) # ------- Backend Definitions -------
abstract type AbstractModelBackend end
struct OptimalControlBackend <: AbstractModelBackend end
struct JuMPBackend <: AbstractModelBackend end
# backend dictionary
backends = Dict(:optimalcontrol => OptimalControlBackend, :jump => JuMPBackend)
# weak dependencies
weakdeps = Dict(OptimalControlBackend => :CTBase, JuMPBackend => :JuMP)
# default backend
__model_backend_key() = :optimalcontrol
# path to problems
path = joinpath(dirname(@__FILE__), "OptimalControlModels")
# ------- Problem Definitions -------
files = filter(x -> x[(end - 2):end] == ".jl", readdir(path))
for file in files
if file ≠ "OptimalControlModels.jl"
problem = Symbol(file[1:end-3])
code = quote
function $problem(; model=__model_backend_key())
model_ = Symbol(lowercase(string(model)))
model_ ∈ keys(backends) ? $problem(backends[model_]()) : error("Unknown model: ", model)
end
function $problem(model_backend::T, args...; kwargs...) where {T <: AbstractModelBackend}
throw(ExtensionError(weakdeps[T]))
end
end
eval(code)
end
end |
Well, to be honest, I think this new version is way too complicated :) I am still convinced that we should stick to multiple dispatch. |
It was just for fun 😆 Extensions is the most important for me. We can do multiple dispatch or let like it is, both are fine for me. Between multiple dispatch or qualify the functions I have no preference. |
Honestly, I'm having a hard time understanding how does the extension work. For this example :
I tried to do the same with the
[weakdeps]
JuMP = "4076af6c-e467-56ae-b986-b466b2749572"
OptimalControl = "5f98b655-cc9a-415a-b60e-744165666948"
[extensions]
JuMPModels = ["JuMP"]
OptimalControlModels = ["OptimalControl"]
module OptimalControlProblems
abstract type AbstractModelBackend end
struct OptimalControlBackend <: AbstractModelBackend end
struct JuMPBackend <: AbstractModelBackend end
backends = Dict(:optimalcontrol => OptimalControlBackend, :jump => JuMPBackend)
weakdeps = Dict(OptimalControlBackend => :OptimalControl, JuMPBackend => :JuMP)
function cart_pendulum(; model=:optimalcontrol)
if model == :optimalcontrol
cart_pendulum(OptimalControlBackend())
elseif model == :jump
cart_pendulum(JuMPBackend())
else
error("unknown model: ", model)
end
end
function cart_pendulum(model_backend::T, args...; kwargs...) where {T <: AbstractModelBackend}
end
end
module OptimalControlModels
using OptimalControlProblems
function OptimalControlProblems.cart_pendulum(model_backend::OptimalControlBackend;nh::Int=100)
println("OK")
end
end and to test it I run : import Pkg
Pkg.activate(".")
using OptimalControlProblems
nlp = OptimalControlProblems.cart_pendulum(; model=:optimalcontrol) And what I didn't understand is how the module of OptimalControlProblems calls |
@0Yassine0 Point demain 9h. |
We could make two extensions containing respectively
JuMPModels
andOptimalControlModels
.The module
JuMPModels
would be triggered byusing JuMP
andOptimalControlModels
byOptimalControl
.This would reduce what is loaded for instance if a user only wants to load
OptimalControl
models.Note
CTBasePlots.jl is an extension of
CTBase.jl
. To trigger it, see the Project.toml.The text was updated successfully, but these errors were encountered: