-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
add rebuild function #1088
Comments
|
I thought the whole discussion we just had in the other issue was that it is deferred to integrator initialization, so passing a fully specified set of independent parameters and initial conditions to ODEProblem does not result in a problem storing the same u0 and p as calling remake with the same input? |
Yes it needs integrator initialization because generally the
The issue though is that it's not really well-defined what the "right" value would be for a Is the plan to effectively then call |
I understand why one needs initialization. I don't understand why For the past year the advice to users has been, if you need to call |
Just to be clear; we are talking about the parameters representing conserved constants that arise from linear conservation laws among species, all of which must be defined via ODEs (not DAEs). How would our manually evaluating and setting such a We would now simply be initializing the value of such |
In general solving for Now with the conservation laws and DAEs you have here, it just so happens that all of the DAE equations are linear and all of the relationships between the
I see, that's a bit of an unfortunate. How common is that in the codebase and users? For all DAEs it won't generally give the derivative but a mix of derivatives and algebraic residuals so it's really only a useful call on ODEs. |
I have no idea. Prior to the past year, MTK itself told people to use I thought one of the goals of MTK was to be composable with many Julia libraries, not just those in SciML? What if one wants to use an optimization package outside of Optimization.jl? Or a non-wrapped dynamical systems analysis library? Does initialization even work for all kinds of SciML integrators, or does it require them to opt into it? While making things work for DAEs is great new functionality, there is a large world of non-DAE models and tooling for which it would also be great to still have a relatively simple way to pass MTK generated functions and parameters. The BifurcationKit extension just directly builds a NonlinearFunction that is passed on -- it doesn't use any integrator initialization to setup guesses or parameters. That is an extension for which one would very much want to use a Catalyst system where conservation laws have been eliminated to ensure a non-singular Jacobian. (And that thankfully still works since the extension uses the older
Yes, I can see that and understand the broader issue in the context of DAEs and callback initialization. However, we are not discussing the general situation. The parameters we are discussing are introduced internally by Catalyst after system creation and are solely functions of species that are all defined via ODEs. A user can't use them in equations / initial conditions / callbacks / defining other parameters as they don't exist at the time the user is building their system. (We have even discussed attaching metadata to them to make clear they have a special meaning.) Since integrator initialization with directly constructed |
I have to admit things are going around a bit in my mind with what should be possible and what shouldn't be possible. However, there are a couple of cases where
generates the wrong simulation. I think we more or less know which cases these are. So, for a starter, creating a |
Those have all been fixed now, right? Can you share any cases that are not fixed? My understanding is that all of those issues are now closed and your cases are now test cases. |
It is, how is it not? MTK is still compatible with the whole list. Though in order to support more advanced features, we do need to make sure that more solvers get updated to support the more advanced features. That said OrdinaryDiffEq is really the only set of solvers for which mass matrix DAEs are supported, except a few in ODEInterface.jl which are a bit slow (and will appropriately error in this case, and we should add the initializeprob/initializeprobmap handling when we get the time).
I don't see the issue here?
Only Sundials and OrdinaryDiffEq have full support for DAEs, and both support all of MTK. What is the issue here?
Agreed. And we still do that? In fact, a lot of the recent changes were required for cases that @TorkelE put together for Catalyst.
We should improve that. @AayushSabharwal can you make sure the problem building here correctly uses the initializeprobmap? The homotopy continuation piece as well. BTW Catalyst likely doesn't need the HomotopyContinuation extension with the MTK level one now?
It doesn't work for implicitly defined parameters, like I discussed above, which is why we're trying to move away from it. We need to put correctness first.
And Catalyst.jl where you define parameters in terms of other parameters, as coming from @TorkelE's examples that drove this in the first place. I don't understand the argument: this is required even for just ODEs, even for Catalyst to be correct! How can you resolve a nonlinear system of equations without calling a solver for a nonlinear system of equations? You can end up in that case even in Catalyst, as shown from the issues that drove the development in the first place!
The explicit cases, but it still gets the implicit cases wrong so we shouldn't rely on that. If you want to do this right, then what you'd do is: https://github.com/SciML/OrdinaryDiffEq.jl/blob/master/lib/OrdinaryDiffEqCore/src/initialize_dae.jl#L152-L191. In fact, it may make sense to move OverrideInit and CheckInit to DiffEqBase so that the code can more easily be moved. But basically what you want to do is solve the |
As for
Sure.
The homotopy continuation interface is purely a |
But nonlinearproblem would have to resolve the parameters? So would homotopy continuation then? |
So perhaps what is needed is detection if everything can be evaluated explicitly, and if so that can be done without having to have the full initialization process? It seems like that is what |
Here is a simple example showing that initialization is not working except in a newly created ODEProblem: using Catalyst, SciMLBase, OrdinaryDiffEqTsit5, Test
rn = @reaction_network begin
(k1,k2), A <--> B
end
osys = convert(ODESystem, rn; remove_conserved = true)
osys = complete(osys)
oprob = ODEProblem(osys, [osys.A => 1.0, osys.B => 1.0], (0.0, 1.0), [osys.k1 => 2.0, osys.k2 => 3.0])
sol = solve(oprob, Tsit5())
# these pass
@test sol(1.0; idxs = osys.A + osys.B) ≈ 2
@test sol(1.0; idxs = osys.Γ[1]) ≈ 2
oprob2 = remake(oprob; u0 = [osys.B => 5.0])
sol2 = solve(oprob2, Tsit5())
# these fail
@test sol2(1.0; idxs = osys.A + osys.B) ≈ 6.0
@test sol2(1.0; idxs = osys.Γ[1]) ≈ 6.0
oprob3 = ODEProblem(osys, [osys.A => 1.0, osys.B => 5.0], (0.0, 1.0), [osys.k1 => 2.0, osys.k2 => 3.0])
sol3 = solve(oprob3, Tsit5())
# these pass
@test sol3(1.0; idxs = osys.A + osys.B) ≈ 6.0
@test sol3(1.0; idxs = osys.Γ[1]) ≈ 6.0
oprob4 = remake(oprob; u0 = [osys.B => 5.0], use_defaults = true)
sol4 = solve(oprob4, Tsit5())
# these fail
@test sol4(1.0; idxs = osys.A + osys.B) ≈ 6.0
@test sol4(1.0; idxs = osys.Γ[1]) ≈ 6.0 |
oprob5 = remake(oprob; u0 = [osys.B => 5.0], p = [osys.Γ[1] => 6.0])
sol5 = solve(oprob5, Tsit5())
# these pass
@test sol5(1.0; idxs = osys.A + osys.B) ≈ 6.0
@test sol5(1.0; idxs = osys.Γ[1]) ≈ 6.0 So if we just wrapped edit: I guess it hasn't been a year since this particular issue was discovered, only six months, so I take that back. My apologies. I was conflating it with various other things that broke last fall and have since been fixed or changed. |
And here is what NonlinearSolve does nsys = convert(NonlinearSystem, rn; remove_conserved = true)
nsys = complete(nsys)
nprob = NonlinearProblem(nsys, [nsys.A => 1.0, nsys.B => 1.0], [nsys.k1 => 2.0, nsys.k2 => 3.0])
sol = solve(nprob)
# these pass
@test sol[nsys.A + nsys.B] ≈ 2
@test sol.ps[nsys.Γ[1]] ≈ 2
nprob2 = remake(nprob; u0 = [nsys.B => 5.0])
sol2 = solve(nprob2)
# these fail
@test sol2[nsys.A + nsys.B] ≈ 6.0
@test sol2.ps[nsys.Γ[1]] ≈ 6.0
nprob3 = NonlinearProblem(nsys, [nsys.A => 1.0, nsys.B => 5.0], [nsys.k1 => 2.0, nsys.k2 => 3.0])
sol3 = solve(nprob3)
# these pass
@test sol3[nsys.A + nsys.B] ≈ 6.0
@test sol3.ps[nsys.Γ[1]] ≈ 6.0
nprob4 = remake(nprob; u0 = [nsys.B => 5.0], use_defaults = true)
sol4 = solve(nprob4)
# these fail
@test sol4[nsys.A + nsys.B] ≈ 6.0
@test sol4.ps[nsys.Γ[1]] ≈ 6.0
nprob5 = remake(nprob; u0 = [nsys.B => 5.0], p = [nsys.Γ[1] => 6.0])
sol5 = solve(nprob5)
# these pass
@test sol5[nsys.A + nsys.B] ≈ 6.0
@test sol5.ps[nsys.Γ[1]] ≈ 6.0 |
We don't have initializeprob infrastructure anywhere except for ODEs/DAEs though. I can add that, but then every solver library needs to hook into it?
There's two parts to the problem here:
Also, could you check the retcode of the failing ODE solves? I have a feeling it'll be |
Thanks for the suggestions. Trying to use guesses here are the updated results -- none seem to work (I assume NonlinearProblem will have analogous issues). I didn't try passing guesses to the system during construction as that isn't a use-case that we can handle seemlessly for users (i.e. Catalyst won't necessarily know the appropriate type information to pass a guess to the system). I tried passing guesses to using Catalyst, SciMLBase, OrdinaryDiffEqTsit5, Test, NonlinearSolve
rn = @reaction_network begin
(k1,k2), A <--> B
end
osys = convert(ODESystem, rn; remove_conserved = true)
osys = complete(osys)
oprob = ODEProblem(osys, [osys.A => 1.0, osys.B => 1.0], (0.0, 1.0), [osys.k1 => 2.0, osys.k2 => 3.0])
sol = solve(oprob, Tsit5())
# these pass
@test sol(1.0; idxs = osys.A + osys.B) ≈ 2
@test sol(1.0; idxs = osys.Γ[1]) ≈ 2
oprob2 = remake(oprob; u0 = [osys.B => 5.0])
sol2 = solve(oprob2, Tsit5())
# these fail
@test sol2(1.0; idxs = osys.A + osys.B) ≈ 6.0
@test sol2(1.0; idxs = osys.Γ[1]) ≈ 6.0
oprob3 = ODEProblem(osys, [osys.A => 1.0, osys.B => 5.0], (0.0, 1.0), [osys.k1 => 2.0, osys.k2 => 3.0])
sol3 = solve(oprob3, Tsit5())
# these pass
@test sol3(1.0; idxs = osys.A + osys.B) ≈ 6.0
@test sol3(1.0; idxs = osys.Γ[1]) ≈ 6.0
oprob4 = remake(oprob; u0 = [osys.B => 5.0], use_defaults = true)
sol4 = solve(oprob4, Tsit5())
# these fail
@test sol4(1.0; idxs = osys.A + osys.B) ≈ 6.0
@test sol4(1.0; idxs = osys.Γ[1]) ≈ 6.0
oprob5 = remake(oprob; u0 = [osys.B => 5.0], p = [osys.Γ[1] => 6.0])
sol5 = solve(oprob5, Tsit5())
# these pass
@test sol5(1.0; idxs = osys.A + osys.B) ≈ 6.0
@test sol5(1.0; idxs = osys.Γ[1]) ≈ 6.0
# guesses not defined for remake
oprob6 = remake(oprob; u0 = [osys.B => 5.0], guesses = [osys.Γ[1] => 1.0])
# errors about invalid kwarg
sol6 = solve(oprob6, Tsit5())
oprob7 = ODEProblem(osys, [osys.A => 1.0, osys.B => 5.0], (0.0, 1.0),
[osys.k1 => 2.0, osys.k2 => 3.0], guesses = [osys.Γ[1] => 1.0])
sol7 = solve(oprob7, Tsit5())
# these pass
@test sol7(1.0; idxs = osys.A + osys.B) ≈ 6.0
@test sol7(1.0; idxs = osys.Γ[1]) ≈ 6.0
oprob8 = remake(oprob7; u0 = [osys.B => 7.0])
sol8 = solve(oprob8, Tsit5())
# errors
@test sol8(1.0; idxs = osys.A + osys.B) ≈ 8.0
@test sol8(1.0; idxs = osys.Γ[1]) ≈ 8.0
oprob9 = remake(oprob7; u0 = [osys.B => 7.0])
# errors about invalid kwarg
sol9 = solve(oprob9, Tsit5(); guesses = [osys.Γ[1] => 1.0])
oprob10 = remake(oprob7; u0 = [osys.B => 7.0], use_defaults = true)
sol10 = solve(oprob10, Tsit5())
# errors
@test sol10(1.0; idxs = osys.A + osys.B) ≈ 8.0
@test sol10(1.0; idxs = osys.Γ[1]) ≈ 8.0 All the cases where |
I'm not sure why |
Proving explicit can be really difficult in general though. We don't have a good cycle detection for that. And then you also cannot make good guarantees either, since if someone does make a cycle then you would have to run the solve to resolve the parameters (and for a nonlinear problem, it is part of the nonlinear solve). Currently the cycle detection uses structural simplify and should make sure that we reuse some of the internals there for pre-simplification if possible?
See what I mentioned above about that. NonlinearProblem, OptimizationProblem, BifurcationProblem, HomotopyProblem etc. all need to get some upgrades in order to ensure correctness with respect to these kinds of constraints. For NonlinearProblem, it would either need to be a two-stage nonlinear solve with a pre-solve for the parameters and then a solve for the state, or you could lift the parameters into the state and let it fully simplify together. The pre-solve is likely more efficient though for the same reason SCCs are. For Optimization, you could just extend the constraint system and lift parameters into the state. For HomotopyProblem and BifurcationProblem, you can use the same initializationproblem pieces and run that prior to BifurcationProblem/HomotopyProblem. |
I don't see where you've provided guesses (except at the very end, to
@ChrisRackauckas this is an interesting case.
Because not every use case needs or wants to update |
Do you mean doing this lifting during simplification, or actually turning them into unknowns? I don't quite get why we'd do the latter since it's an initialization process. Otherwise wouldn't it also make sense to lift them into states for ODEs and the like? Similarly for NonlinearProblems, initialization is not identical to SCCs since we're solving for the initial values of states that are also solved by the actual nonlinear problem, as opposed to it being a disjoint set. |
If passing a quantitatively meaningless guess to a system solves this problem -- anything we pass would be no better than calling However, more generally we need the remake process in this case to work for all problem types, not just ODESystems that sit on top of integrators that opt into initialization. Otherwise we will experience the same issues with other problem types. FWIW, whatever is being done in |
Well there's a few things that we could do. I do think it would be nice if all explicit relationships could be resolved immediately. That's always going to be slower than using a generated function though since it would require doing the symbolic substitution which is a slow interpreted process. Also, it would only apply to explicit relationships, so we'd need to error if there's any algebraic loops.
That's different, because the ODE is not a single implicit system but instead N+1 implicit systems.
It is a manual SCC decomposition. You are solving for the So it is equivalent. The reason to do it as two separate steps would just be an optimization over that. |
It isn't always meaningless; the guess can influence the result of the nonlinear solve. All zero guesses cause singularities. We require a guess for parameters for two main reasons:
You can also opt-in to solving for a parameter by giving it a default of
What assumptions are you referring to here? The type of a parameter is always known. We validate this in MTK. You can't give a scalar parameter an array value, or vice versa, etc.
|
So for time-independent types, initialization should only solve for parameters? |
and initial guesses. |
I was just mentioning that presolving for initial guesses can be made equivalent to a parameter solve and a linear shift in the equations + SCC decomposition. |
I see. Yeah that makes sense. Thanks for the clarification. |
Since
remake
won't handle updating conserved constants in problems by design, we can make our own wrapper over it we tell Catalyst users to use that will be more user friendly, (sayrebuild
).All it needs to do is wrap
remake
, check if a user is updating a species a conserved constant depends on, and then calculate the new constant value. Then we callremake
adding the new constant value into the updatedp
passed by the user (or making an updatedp
if none is passed). If a user passes a manual value for the constant we can just forward everything toremake
directly (which seems to work).Then we can update the docs and tests to use
rebuild
instead ofremake
.The text was updated successfully, but these errors were encountered: