Skip to content
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

Problem changing functions #230

Closed
SebastianM-C opened this issue Dec 31, 2017 · 21 comments · Fixed by SciML/DiffEqBase.jl#87
Closed

Problem changing functions #230

SebastianM-C opened this issue Dec 31, 2017 · 21 comments · Fixed by SciML/DiffEqBase.jl#87

Comments

@SebastianM-C
Copy link

An interface for problem changing functions would be useful. It would help when creating MonteCarloProblems for other types of problems than ODEProblems.

@Datseris
Copy link

Datseris commented Jan 24, 2018

This is a great suggestion. For the packages of DynamicalSystems.jl I have set up a small interface internally.
(the following are before version 3)

# ODEProblem helper functions
ODEProblem(ds::ContinuousDS) = ds.prob

ODEProblem(
ds::ContinuousDS, t::Real, state = ds.prob.u0) =
ODEProblem{true}(ds.prob.f, state, (zero(t), t),
callback = ds.prob.callback, mass_matrix = ds.prob.mass_matrix)

ODEProblem(ds::ContinuousDS, tspan::Tuple, state = ds.prob.u0) =
ODEProblem{true}(ds.prob.f, state, tspan,
callback = ds.prob.callback, mass_matrix = ds.prob.mass_matrix)

"""
    ODEProblem(ds::ContinuousDS, t, state = ds.prob.u0 [, callback])
Create a new `ODEProblem` for the given dynamical system that final time `t` (or `tspan`
for tuple `t`), `state` and if given, also merges the `callback` with other existing
callbacks currently in `ds.prob`.
"""
function ODEProblem(ds::ContinuousDS, t::Real, state, cb)
    if ds.prob.callback == nothing
        return ODEProblem{true}(ds.prob.f, state, (zero(t), t),
        callback = cb, mass_matrix = ds.prob.mass_matrix)
    else
        return ODEProblem{true}(ds.prob.f, state, (zero(t), t),
        callback = CallbackSet(cb, ds.prob.callback),
        mass_matrix = ds.prob.mass_matrix)
    end
end

(the ContinuousDS struct has an ODEProblem as its field)

I think it is safe to say that I should just do a PR to DiffEqBase or to whatever repo @ChrisRackauckas instructs, with the above replacing ds with some odeproblem. Then I think one more keyword argument parameters can be added to support the new syntax.

If there exists some function ismutable that checks whether a thing is mutable, the above definitions where one only adds new parameters could also change prob.p in place instead of create a new problem.

edit: It should be debated on whether the parameters are keywords or optional arguments, and whether the keyword should be just p or parameters.

@ChrisRackauckas
Copy link
Member

Using dispatch to distinguish the cases here would be prone to bugs. For example, DynamicalODEProblems use a tuple for the u0 all the time, so it would catch the wrong route here. I think we need a few functions with clear succinct names.

@Datseris
Copy link

Datseris commented Jan 24, 2018

You mean that I should not use ODEProblem as the function name since the "Problem Type" is also tied to the struct, something which I completely disregarded here? Then you are right.

@Datseris
Copy link

Datseris commented Jan 24, 2018

Or just make tspan a keyword as well. The method that has only t::Real here is just for my convenience, and doesn't need to belong in the general interface. Maybe that could circumvent the issue.

@ChrisRackauckas
Copy link
Member

I think that problem_new_parameters is too long, but

change_tspan(prob,tspan)
change_ic(prob,u0)
change_prob(prob;u0 = prob.u0, ...)

@Datseris
Copy link

Datseris commented Jan 24, 2018

Hm, but this would mean that one cannot use the same function name to change different aspects of the ODEProblem. I kind of dislike that.

EDIT: But then again, you can just only ever use change_prob as a user. So maybe it is fine.

@ChrisRackauckas
Copy link
Member

ChrisRackauckas commented Jan 24, 2018

What about change_prob just using keyword arguments? On v0.7 that would specialize just fine. It's dispatch on positional arguments that's suspect.

@Datseris
Copy link

Yeah I think that is fine. With everything besides prob to be a keyword argument, right? Seems like the best one so far.

@Datseris
Copy link

Datseris commented Jan 24, 2018

This is the implementation I have written up internally at the moment:

# ODEProblem helper functions
ODEProblem(ds::ContinuousDS) = ds.prob

function ODEProblem(ds::ContinuousDS;
                    t = ds.prob.tspan[end],
                    state = ds.prob.u0,
                    parameters = ds.prob.p,
                    callback = ds.prob.callback,
                    mass_matrix = ds.prob.mass_matrix,
                    tspan = (zero(t), t))
    return ODEProblem(ds.prob.f, state, tspan, parameters,
           mass_matrix = mass_matrix, callback = callback)
end

I've taken the (I think smart) choice to have two keyword arguments t and tspan. This way I can just give a final time and the initial is assumed zero, but also control the full tspan if necessary.

EDIT: I propose this as the canditate for solving the present issue.

(EDIT: obviously replacing ds.prob with simply prob)

@Datseris
Copy link

Here is an accompanying documentation string:

"""
    ODEProblem(ds::ContinuousDS; kwargs...)
Create a new `ODEProblem` for the given dynamical system by changing specific
aspects of the existing `ODEProblem`.

Keyword arguments: `state, t, parameters, callback, mass_matrix, tspan`.

If `t` is given, a `tspan` is created with initial time assumed zero. If `tspan`
is given directly, the keyword `t` is disregarded.
"""

(clearly I'll replace ContinuousDS with ODEProblem if this is accepted)

@tkf
Copy link

tkf commented Feb 13, 2018

Here is an idea to solve this in general with three ingredients. In short, my suggestion is to define keyword-only constructors so that re-creation is easy.

First, let's define an API to extract a constructor of problems and other structs (BTW, is there a way to do this in Julia in more automated way? This sounds silly. I'm too Julia noob...) For basic struct, I guess you can either do some introspection and/or macro to slightly automate this part.

constructor_of(::ODEProblem) = ODEProblem
constructor_of(::DiscreteProblem) = DiscreteProblem
constructor_of(::SDEProblem) = SDEProblem
constructor_of(::SplitFunction) = SplitFunction  # not just problem types!
# ... and so on

Second, let's define keyword-only constructors. Note this requires field names and arguments have to have one-to-one correspondence. Julia 1.0 would make this part easier JuliaLang/julia#25830 :

ODEProblem(;
           f = error("No argument f"),
           u0 = error("No argument u0"),
           tspan = error("No argument tspan"),
           p = nothing,
           kwargs...) =
    ODEProblem(f, u0, tspan, p; kwargs)

Finally, define reset (or maybe some other name, since there's Base.reset):

reset(prob::DEProblem; kwargs...) =
    constructor_of(prob)(;
                         [n => getfield(prob, n) for n in fieldnames(prob)]...,
                         kwargs...)

Note that this still require users to call reset multiple times to reset a "nested" element:

old_prob = SplitODEProblem(...)
new_prob = reset(old_prob; f = reset(old_prob.f; f1 = new_f1))

OK. In this case, by intercepting the constructor mapping by constructor_of(::ODEProblem{..., SplitODEProblem}) = SplitODEProblem, you can do a flat reset. But I'm not sure if this strategy works all the time.

@tkf
Copy link

tkf commented Feb 14, 2018

Oops, the signature reset(prob::DEProblem; kwargs..) has to be more general for it to work for SplitODEProblem. It'd be nice if we can make the first type to be Any, but then we really need to avoid name clash. How about remake?

Also, this is so general and I started to wonder if Parameters.jl or some other libraries already have it.

@tkf
Copy link

tkf commented Feb 14, 2018

Ah, of course, Parameters.reconstruct does that. Why not use Parameters.jl everywhere?

https://mauro3.github.io/Parameters.jl/latest/api.html#Parameters.reconstruct-Union%7BTuple%7BT%2CAny%7D%2C%20Tuple%7BT%7D%7D%20where%20T

@ChrisRackauckas
Copy link
Member

I'm not sure Parameters.jl allows inner constructors, in which case we can't.

@ChrisRackauckas
Copy link
Member

Might as well just make it named the same as the constructor like in @Datseris 's instead of reset. I like his idea. We can try automating it later, but I don't think it's completely necessary at first: it's fine to just get it and clean it later. I don't think automating it would be all that easy.

@tkf
Copy link

tkf commented Feb 14, 2018

I don't think using the constructor as resetting API is a good choice. What happens in the code that I don't know what the problem type is but just want to reset tspan (say)? You need to define some API like constructor_of, right? Or am I missing some Julia feature?

Besides, if users have to call constructor_of(prob)(prob; new_params...) then why not define the function remake does that? (or call it reset/change/reconstruct/...) Furthermore, defining ProblemType(; kwargs...) and ProblemType(prob; kwargs...) take more or less the same amount of effort but the former is re-usable in other contexts like creating a problem given a dictionary.

BTW, full-automation to define constructor_of using reflection/introspection may not be super hard. Here is a quick example that works:

"""
    constructor_of(thing)::Type

Return a type/constructor for `thing`.
"""
function constructor_of end

for n in names(DiffEqBase)
    v = try
        getfield(DiffEqBase, n)
    catch
        continue
    end
    if v isa Type && ! (v isa Union) && isempty(subtypes(v))
        eval(:(constructor_of(::DiffEqBase.$n) = DiffEqBase.$n))
    end
end

@tkf
Copy link

tkf commented Feb 14, 2018

Just for clarity: my suggestion is not about automation. It's about more factorized API (keyword-only constructor and constructor_of) that can make implementation of the problem changing functions nearly trivial.

@tkf
Copy link

tkf commented Feb 15, 2018

Obviously, constructor_of can be implemented in one line :)

constructor_of(::T) where T = getfield(T.name.module, Symbol(T.name.name))

@ChrisRackauckas
Copy link
Member

So you mean, define a generic remake via metaprogramming, and then make things like ODEProblem(prob;kwargs...) = remake(prob;kwargs...)? That would work.

@tkf
Copy link

tkf commented Feb 16, 2018

Can you define a generic remake? I thought the constructor is the only way to constructor a struct (besides copy). But the problem is that it's not clear how to find the right constructor calling signature and/or how to map the fields to the arguments, right? That's why I was suggesting the keyword-only constructor that maps the keywords to fields.

Here is the minimal implementation of my suggestion. Once you have the keyword-only constructor defined somehow, the rest is straightforward:

using Parameters: type2dict
using DiffEqBase
import DiffEqBase: ODEProblem

# Manual implementation of the keyword-only constructor:
ODEProblem(;
           f = error("No argument f"),
           u0 = error("No argument u0"),
           tspan = error("No argument tspan"),
           p = nothing,
           problem_type = StandardODEProblem(),
           kwargs...) =
    ODEProblem(f, u0, tspan, p; kwargs...)

constructor_of(::T) where T = getfield(T.name.module, Symbol(T.name.name))

remake(prob; kwargs...) = constructor_of(prob)(; type2dict(prob)..., kwargs...)


using Base.Test
@testset "remake" begin
    function f(du,u,p,t)
        du[1] = 0.2u[1]
        du[2] = 0.4u[2]
    end
    u0 = ones(2)
    tspan = (0,1.0)

    prob1 = ODEProblem(f, u0, tspan; mass_matrix=eye(length(u0)))
    new_u0 = [2.0, 2.0]
    new_p = Dict()
    prob2 = remake(prob1; u0 = new_u0, p = new_p)
    @test prob1.f === prob2.f
    @test prob1.mass_matrix === prob2.mass_matrix
    @test prob2.u0 == new_u0
    @test prob2.p === new_p
end

Of course, we can instead define "re-constructors" (ODEProblem(prob; kwargs..) etc.) and then remake can be defined as:

remake(prob; kwargs...) = constructor_of(prob)(prob; type2dict(prob)..., kwargs...)

The reason why I thought the implementation based on the keyword-only constructor was better was that it can be used to define the "re-constructors" but the reverse was not true. In a way, the keyword-only constructor is more fundamental. But this isn't a big deal anyway.

I think defining the keyword-only constructor and/or "re-constructors" via macro/introspection are not so hard. I thought defining constructor_of was the biggest obstacle which turned out to be a one-linear.

Anyway, the point I want to emphasize is that remake is more useful than ODEProblem(prob; kwargs..) since the code using remake works for all problem types.

@ChrisRackauckas
Copy link
Member

I am running into a problem where remake would be the perfect solution, so yes let's do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants