-
-
Notifications
You must be signed in to change notification settings - Fork 214
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
Refactor OrdinaryDiffEq to use Subpackages for Solver Sets #2177
Comments
Sounds like an interesting idea 👍
I agree - it's easier if you keep everything in the same repo. Then, it's much easier for people to search for certain algorithms/caches etc. when they change something.
The last sentence is incomplete?
Having a package for explicit solvers only would be interesting for us in Trixi.jl - since we mainly use explicit methods (SSP and low-storage schemes).
Fully agree 👍 Even the least used methods are likely used by somebody.
This would make it much more tedious to experiment with different solvers. I think it's fine to keep similar solvers together.
👍 Just as a general comment: When we do that, we should also keep an eye on the readability of the code - using too many complicated macros etc. can make it much harder to understand the code for newcomers. I prefer having a bit of redundancy if it makes the code easier to read and understand. |
Fully agree on this, but it's also worth balancing this by the fact that redundant code significantly increases the bug/testing surface, and can obscure the differences between different methods. |
That should be caught in tests. |
@time begin
using OrdinaryDiffEqRosenbrock
function lorenz(du, u, p, t)
du[1] = 10.0(u[2] - u[1])
du[2] = u[1] * (28.0 - u[3]) - u[2]
du[3] = u[1] * u[2] - (8 / 3) * u[3]
end
lorenzprob = ODEProblem(lorenz, [1.0; 0.0; 0.0], (0.0, 1.0))
sol = solve(lorenzprob, Rodas5P());
end;
# 1.864242 seconds (3.40 M allocations: 217.743 MiB, 2.05% gc time, 6.16% compilation time: 2% of which was recompilation)
# Prior to the changes
@time begin
using OrdinaryDiffEq
function lorenz(du, u, p, t)
du[1] = 10.0(u[2] - u[1])
du[2] = u[1] * (28.0 - u[3]) - u[2]
du[3] = u[1] * u[2] - (8 / 3) * u[3]
end
lorenzprob = ODEProblem(lorenz, [1.0; 0.0; 0.0], (0.0, 1.0))
sol = solve(lorenzprob, Rodas5P());
end;
# 3.419571 seconds (6.84 M allocations: 444.780 MiB, 4.16% gc time, 45.04% compilation time)
@time begin
using OrdinaryDiffEqTsit5
function lorenz(du, u, p, t)
du[1] = 10.0(u[2] - u[1])
du[2] = u[1] * (28.0 - u[3]) - u[2]
du[3] = u[1] * u[2] - (8 / 3) * u[3]
end
lorenzprob = ODEProblem(lorenz, [1.0; 0.0; 0.0], (0.0, 1.0))
sol = solve(lorenzprob, Tsit5());
end;
# 0.564754 seconds (975.66 k allocations: 72.982 MiB, 1.42% gc time, 15.30% compilation time: 2% of which was recompilation)
# Before
@time begin
using OrdinaryDiffEq
function lorenz(du, u, p, t)
du[1] = 10.0(u[2] - u[1])
du[2] = u[1] * (28.0 - u[3]) - u[2]
du[3] = u[1] * u[2] - (8 / 3) * u[3]
end
lorenzprob = ODEProblem(lorenz, [1.0; 0.0; 0.0], (0.0, 1.0))
sol = solve(lorenzprob, Tsit5());
end;
# 2.459902 seconds (5.46 M allocations: 349.671 MiB, 3.02% gc time, 22.39% compilation time) |
Rosenbrock is kind of tricky. It has to have NonlinearSolve because it needs it if you have a DAE since it needs to run the initialization solve. That is about half of the remaining load time. |
From the timings on
include
:It's clear that this package has a long precompilation time simply because it's big. The suggestion then is to turn many of the solvers into subpackages. These can be subpackages like in a
lib
folder in this repo, kept in the same repository because it is expected that a lot of co-development will still be required between the solvers and the integrator type, but separated so that users can install a form without all of the solvers.Now the question is how to get there? There are some pieces that can very clearly be separated. I think we can do this by classes of algorithms. For example, symplectic integrators, exponential integrators, low storage Runge-Kutta methods, and extrapolation methods are all not on the "main line" of the package, i.e. they are not part of the default algorithm and they are used in a minority of situations. Those clearly could be split to submodule packages and most users would benefit.
The edge case is what to do with methods on the default. The default set is being narrowed a bit with #2103 to just some explicit RK methods, a few Rosenbrock methods, and FBDF. Do those and the default solver stay in OrdinaryDiffEq.jl, or do we move all of those out as well and make another subpackage just for the default solver?
The issue is that we need to make sure we get this "basically right" the first time, because this choice is breaking as some solvers would no longer be exported by default without pulling in new subpackages. The good thing though is that this won't be breaking to DifferentialEquations.jl, which keeps the simple users on a simpler update track.
Some questions to ask and answer:
perform_step!
,tableau
,cache
, etc. implementations are all relying on internals of this package. It would be unstable to keep them separate as any change to these internals would suddenly become breaking, and we expect from history that this is relatively common. To make this simpler to maintain, we can have the subpackages inlib
folders so they can easily be bumped with any internals.solve.jl
front end type promotions and callbacks, which are reused by things like Sundials. OrdinaryDiffEq core is only forSeeking comments from @devmotion @YingboMa @ranocha @KristofferC @oscardssmith
The text was updated successfully, but these errors were encountered: