-
-
Notifications
You must be signed in to change notification settings - Fork 116
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
DiffEqBase is not "lightweight" #618
Comments
It has definitely grown over time. The problem is that it now serves two purposes. It now gives the interface and the standard implementations for many parts of the interface. For example, it makes sense to have the interface for all of the callbacks be together: https://github.com/SciML/DiffEqBase.jl/blob/master/src/callbacks.jl#L1-L379 but then a lot of the solvers make use of the common callback implementation: https://github.com/SciML/DiffEqBase.jl/blob/master/src/callbacks.jl#L381-L898 That implementation is what introduces the dependencies for the nonlinear solvers. Then there's a linear and nonlinear common interface for choosing how to overload portions of the DifferentialEquation solver. But that's used everywhere, so it wouldn't make sense to put those helper functions in OrdinaryDiffEq, so pre-built linear solver helper functions live in DiffEqBase: https://github.com/SciML/DiffEqBase.jl/blob/master/src/linear_nonlinear.jl#L1-L241 and that's where the IterativeSolvers.jl dependency comes in. At the same time, SciML has moved beyond DiffEq. https://github.com/SciML/DiffEqBase.jl/blob/master/src/problems/basic_problems.jl is a clear example of this, where it all exists for things going on in ModelingToolkit, NeuralPDE, DiffEqOperators, etc. So there's What this is telling me is that we need to split the interface portion from the I think I've mentioned this problem to @kanav99 before, but haven't moved too much on it yet. |
There are many reasons for this. 1. DiffEqBase.jl has grown to be a base between the differential equation solvers, with common tools for nonlinear solvers and callbacks adding a ton of dependencies. Users like @mauro3 noticed this in SciML/DiffEqBase.jl#618 2. A lot of DiffEqBase was becoming unrelated to differential equations. OptimizationProblem, NonlinearProblem, all of those functions, etc. It's a full interface for SciML, not DiffEq which is by now a subset of the full ecosystem. But it needed to play the Base role, which meant that nonlinear problems were defined in the repo carrying all of the dependencies for the ODE solver's callback system. 3. It started inviting circular dependencies. Some of these problems are pervasive. We need to get the simplest form to GalacticOptim.jl, Quadrature.jl, NonlinearSolve.jl, etc. which are becoming the DiffEq's of their respective regimes (though there's not as much to do in those areas). But this meant NonlinearSolve.jl needed NonlinearProblem, but NonlinearSolve.jl was used in the callbacks, so... it had its own problem type. This gives an issue to a system like MTK which then has no unique type to target. 4. There's a lot of churn on the DiffEq solver portions of DiffEqBase, but relatively little on the actual interface portions. So it makes sense to let them be more stable. 5. There's a ton of packages which are @requires in DiffEqBase, which adds to the compile time. This was done because the library was supposed to avoid dependencies, but as the central piece of the differential equation solvers it really couldn't, and so it used required to fake lower dependencies but some of those like ForwardDiff.jl should really be promoted to being a real dependency now that it doesn't have the "idea" of being no-dependency. For backwards compatibility, DiffEqBase reexports SciMLBase and imports a ton of the abstract types it used to have. This should allow for the migration to take place over time, while still fixing some of the MTK and NonlinearSolve issues that need a fix ASAP. DiffEqBase will have a companion PR to match it. There are some remaining issues here though: 1. I would like to get rid of the LabelledArrays dependency. Is there a good generic to use for the names? `nameof`? 2. Is the split at the right level? 3. I want to drop the Queryverse stuff like TableTraits and IteratorInterfaceExtensions since we have Tables.jl
Solved. Depend on SciMLBase for the truly lightweight one. |
And the related discourse post: https://discourse.julialang.org/t/psa-the-right-dependency-to-reduce-from-differentialequations-jl/72757/2 Thanks Chris! |
The "About" says that DiffEqBase is lightweight, but in IMO it's not:
(on my fast laptop)
Not sure what to do here, nor do I expect any solution (nor could I provide one: so feel free to close). However I thought to put it out there as it would be nice if DiffEqBase was so lightweight that including it as a dep would be a decision not requiring any consideration.
The text was updated successfully, but these errors were encountered: