Summary of breakages when updating problems with conservation laws

[TorkelE]

So at this stage I am starting to be uncertain exactly what is intended to work and what is not intended to work. At least here is a summary of the tests I have broken, and which works and which do not work. There are definitely some oddities going on here.

# Create model and fetch the conservation parameter (Γ).
t = default_t()
@parameters k1 k2
@species X1(t) X2(t)
rxs = [
    Reaction(k1, [X1], [X2]),
    Reaction(k2, [X2], [X1])
]
@named rs = ReactionSystem(rxs, t)
osys = convert(ODESystem, complete(rs); remove_conserved = true)
osys = complete(osys)
@unpack Γ = osys

# Creates an `ODEProblem`.
u0 = [X1 => 1.0, X2 => 2.0]
ps = [k1 => 0.1, k2 => 0.2]
oprob = ODEProblem(osys, u0, (0.0, 1.0), ps)

# Check `ODEProblem` content.
oprob[X1] == 1.0
oprob[X2] == 2.0
oprob.ps[k1] == 0.1
oprob.ps[k2] == 0.2
oprob.ps[Γ[1]] == 3.0

# Attempts to update problem  by giving new values for `X2` (broken)
@test_broken oprob[X2] = 20.0
@test_broken oprob_new = remake(oprob; u0 = [X1 => 10.0, X2 => 20.0])

# Updates problem using `remake` and check that the new values are correct.
oprob_new = remake(oprob; u0 = [X1 => 10.0])
@test oprob_new[X1] == 10.0
@test_broken oprob_new[X2] == 2.0 # Currently -7.
@test_broken oprob_new.ps[Γ[1]] == 12.0 # Currently 3.0.
integrator = init(oprob_new, Tsit5())
@test integrator[X1] == 10.0
@test_broken integrator[X2] == 2.0 # Currently -7.
@test_broken integrator.ps[Γ[1]] == 12.0 # Currently 3.0

# Updates problem using normal indexing (uncertain exactly what intended behaviour here should be).
oprob[X1] = 10.0
@test oprob[X1] == 10.0
@test_broken oprob[X2] == 2.0 # Currently -7.
@test_broken oprob.ps[Γ[1]] == 12.0 # Currently 3.0.
integrator = init(oprob, Tsit5())
@test integrator[X1] == 10.0
@test_broken integrator[X2] == 2.0 # Currently -7.
@test_broken integrator.ps[Γ[1]] == 12.0 # Currently 3.0.

I also have some general MTK tests, where remake actually works now (except that I thought that it wasn't meant to any longer).

# Checks that initial conditions/paraemters dpending on other initial conditions/parameters
# as defaults are updated correctly.
# Checks using normal indexing and `remake`.
# Checs effect on updated problem and integrators initiatedfrom it.
# An issue discussing what should, and should not, work is here: https://github.com/SciML/ModelingToolkit.jl/issues/2733
let
    # Creates the `ReactionSystem` (with defaults and an observable).
    @parameters k1
    @species X1(t)
    @parameters k2 = X1
    @species X2(t) = k1
    @variables O(t)
    rxs = [
        Reaction(k1, [X1], [X2]),
        Reaction(k2, [X2], [X1])
    ]
    observed = [O ~ X1 + X2 + k1 + k2]
    @named rs = ReactionSystem(rxs, t; observed)
    rs = complete(rs)

    # Creates the various problem types.
    u0 = [X1 => 1]
    ps = [k1 => 10]
    oprob = ODEProblem(rs, u0, (0.0, 1.0), ps)
    sprob = SDEProblem(rs, u0, (0.0, 1.0), ps)
    dprob = DiscreteProblem(rs, u0, (0.0, 1.0), ps)
    jprob = JumpProblem(rs, dprob, Direct())
    nprob = NonlinearProblem(rs, u0, ps)
    @test_broken false # ssprob = SteadyStateProblem(rs, u0, ps) # Cannot generate integrators from `SteadyStateProblem`s (https://github.com/SciML/SteadyStateDiffEq.jl/issues/79).
    probs = [oprob, sprob, jprob, nprob, ssprob]
    solvers = [Tsit5(), ImplicitEM(), SSAStepper(), NewtonRaphson(), DynamicSS(Tsit5())]

    # Checks that values depending on defaults are updated correctly. Checks values both in original
    # problem and in the initialised integrator. Only uses single symbolic when indexing (other
    # alternatives should have been checked elsewhere).
    for (prob, solver) in zip(deepcopy(probs), solvers)
        # Checks when updating using `remake` indexing. I *think* this *should* update values (in the
        # updated problem) that depend on default (the integrator should have the correct values).
        prob_new = remake(prob; u0 = [X1 => 2], p = [k1 => 20])
        @test prob_new[X1] == 2
        @test prob_new[X2] == 20
        @test prob_new.ps[k1] == 20
        @test prob_new.ps[k2]  == 2
        @test prob_new[O] == 44
        integrator = init(prob_new, solver)
        @test integrator[X1] == 2
        @test integrator[X2] == 20
        @test integrator.ps[k1] == 20
        @test integrator.ps[k2] == 2
        @test integrator[O] == 44

        # Checks when updating using normal indexing. I *think* this *should not* update values (in
        # the updated problem) that depend on default (the integrator should have the correct values).
        prob[X1] = 3
        prob.ps[k1] = 30
        @test prob[X1] == 3
        @test prob[X2] == 10
        @test prob.ps[k1] == 30
        @test prob.ps[k2] == 1
        @test prob[O] == 44
        integrator = init(prob, solver)
        @test integrator[X1] == 3
        @test_broken integrator[X2] == 30
        @test integrator.ps[k1] == 30
        @test_broken integrator.ps[k2] == 3
        @test_broken integrator[O] == 66
    end
end

[ChrisRackauckas]

X2 is algebraic and simplified out, right? So it doesn't have an initial condition since it's not a state. You can give it a parameter for the initialization, though in this kind of case it's generally much better to not over-specify initial conditions anyways, i.e. you want the user to not supply an initial condition for X2.

[isaacsas]

There is no way we can auto-determine the value of the new parameter associated with a conservation law without having initial values for all pre-elimination species.

[ChrisRackauckas]

If x + y = 1 and you tell me that x = 1, then I know that y = 0. If you specify that x = 1 and y = 1 I will tell you that you cannot do that because that makes x + y = 1 impossible. That's what's going on here.

[isaacsas]

But in our case Γ := x(t) + y(t) with Γ a new unknown parameter. We want to use x(0) and y(0) to set the value of Γ.

[ChrisRackauckas]

Γ would need to be made a dependent parameter then, built from the initial conditions of the other two variables (which would need to be made into parameters) and then that would need to use the initialization system.

[isaacsas]

But doesn't that mean users will have to be told that if they use Catalyst they have to explicitly specify parameters to represent all initial conditions? We have no way of knowing a priori which unknowns will appear in conservation laws, and we wouldn't want a situation where we sometimes inject new parameters in the background and then expect users to know about them and switch to setting initial conditions using them...

[TorkelE]

I might be misunderstanding, but weren't dependent parameters which were just expressions of other parameters? I.e. basically a way to write a commonly occurring parameter expression in a shorter form as a single symbol? I think this case is something else, i.e. a parameter which value is being initiated according to some other values.

The funny thing is that everything works fine, it is just the case when the problem is updated that there is a problem, but the initial creation is all good.

[isaacsas]

@TorkelE what Chris was suggesting is we need to make the initial conditions parameters too for each species appearing within a conservation law, and have users update those parameters to change initial conditions.

[isaacsas]

i.e.

@parameters A0
@species A(t)=A0

and then users update A0 to change the initial condition.

[ChrisRackauckas]

No user's don't need to do that, Catalyst would need to. Or @AayushSabharwal do we support initial values in dependent parameters?

[AayushSabharwal]

Yeah we do.

@variables x(t)
@parameters p = x

will initialize p to x if a value is not explicitly provided.

The reverse works as well.

[isaacsas]

But wouldn't Catalyst adding parameters to represent initial conditions mean that any user using downstream functionality (like remake) would have to know and always update initial conditions via changing these parameters? Moreover, everything would then break silently should a user directly update an initial condition instead of updating it via its parameter.

Doesn't it also mean that there are possibly large numbers of extra parameters hanging around for users working on higher level analyses (sensitivity analysis, etc)? This seems undesirable.

[ChrisRackauckas]

Since using initial conditions as parameters like Aayush shows works, making a dependent parameter Γ ~ x + y should be all that's needed?

[isaacsas]

My understanding from @TorkelE is that when subsequently remaking a problem it won’t be correctly recalculated as described above.