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

Use SciMLBase over OrdinaryDiffEq where possible #2074

Merged
merged 3 commits into from
Sep 8, 2023
Merged

Use SciMLBase over OrdinaryDiffEq where possible #2074

merged 3 commits into from
Sep 8, 2023

Conversation

charleskawczynski
Copy link
Member

This PR is an attempt to remove our dependency on OrdinaryDiffEq, in favor of using the lighter weight package, SciMLBase.

Unfortunately, we can't remove the dependency because, for example, ODE.Tsit5() is an ODE algorithm that is (I think) first defined in OrdinaryDiffEq. Same goes for a bunch of methods defined in type_getters.jl:

is_explicit_CTS_algo_type(alg_or_tableau) =
    alg_or_tableau <: CTS.ERKAlgorithmName

is_imex_CTS_algo_type(alg_or_tableau) =
    alg_or_tableau <: CTS.IMEXARKAlgorithmName

is_implicit_type(::typeof(ODE.IMEXEuler)) = true
is_implicit_type(alg_or_tableau) =
    alg_or_tableau <: Union{
        ODE.OrdinaryDiffEqImplicitAlgorithm,
        ODE.OrdinaryDiffEqAdaptiveImplicitAlgorithm,
    } || is_imex_CTS_algo_type(alg_or_tableau)

is_ordinary_diffeq_newton(::typeof(ODE.IMEXEuler)) = true
is_ordinary_diffeq_newton(alg_or_tableau) =
    alg_or_tableau <: Union{
        ODE.OrdinaryDiffEqNewtonAlgorithm,
        ODE.OrdinaryDiffEqNewtonAdaptiveAlgorithm,
    }

is_imex_CTS_algo(::CTS.IMEXAlgorithm) = true
is_imex_CTS_algo(::SciMLBase.AbstractODEAlgorithm) = false

is_implicit(::ODE.OrdinaryDiffEqImplicitAlgorithm) = true
is_implicit(::ODE.OrdinaryDiffEqAdaptiveImplicitAlgorithm) = true
is_implicit(ode_algo) = is_imex_CTS_algo(ode_algo)

is_rosenbrock(::ODE.Rosenbrock23) = true
is_rosenbrock(::ODE.Rosenbrock32) = true
is_rosenbrock(::SciMLBase.AbstractODEAlgorithm) = false
use_transform(ode_algo) =
    !(is_imex_CTS_algo(ode_algo) || is_rosenbrock(ode_algo))

additional_integrator_kwargs(::SciMLBase.AbstractODEAlgorithm) = (;
    adaptive = false,
    progress = isinteractive(),
    progress_steps = isinteractive() ? 1 : 1000,
)
additional_integrator_kwargs(::CTS.DistributedODEAlgorithm) = (;
    kwargshandle = ODE.KeywordArgSilent, # allow custom kwargs
    adjustfinal = true,
    # TODO: enable progress bars in ClimaTimeSteppers
)

is_cts_algo(::SciMLBase.AbstractODEAlgorithm) = false
is_cts_algo(::CTS.DistributedODEAlgorithm) = true

A bunch of these ODE. types are (I think) first defined in OrdinaryDiffEq.jl.

@charleskawczynski
Copy link
Member Author

Increasing the jet failures is kind of a bummer, I'm going to see if I can break this down a bit to avoid that, or at least see where it's happening.

@charleskawczynski charleskawczynski marked this pull request as draft September 8, 2023 14:24
@charleskawczynski charleskawczynski marked this pull request as ready for review September 8, 2023 16:22
@charleskawczynski
Copy link
Member Author

Actually, it looks like load times have improved quite a bit. Maybe it's a fluke, but I think this is generally headed in the right direction if we eventually remove OrdinaryDiffEq in favor of SciMLBase (which we can get away with once we convert the indefinite integrals that we currently depend on: search for ODE.solve( to find).

@charleskawczynski
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 8, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 4205c01 into main Sep 8, 2023
6 checks passed
@bors bors bot deleted the ck/scimlbase branch September 8, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant