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

Integrate SciMLOperators with rest of the ecosystem #142

Open
12 of 19 tasks
vpuri3 opened this issue Jan 30, 2023 · 18 comments
Open
12 of 19 tasks

Integrate SciMLOperators with rest of the ecosystem #142

vpuri3 opened this issue Jan 30, 2023 · 18 comments

Comments

@vpuri3
Copy link
Member

vpuri3 commented Jan 30, 2023

Previous attempts to integrate SciMLOperators into the ecosystem broke everything. This was because we coupled together two separate tasks:

  1. supporting SciMLOperators
  2. deprecating AbstractDiffEqOperator, DiffEqLinearOperator, AbstractDiffEqCompositeOperator, and the concrete types.

So we were never able to get anywhere because errors (in downstream packages) from the deprecations in step 2 were too many to handle. This time we shall do the integration step-by-step.

  • Define all operators and traits in SciMLOperators
  • Have SciMLBase reexport SciMLOperators. Do NOT deprecate diffeqoperator family
  • Ditto for DiffEqBase.jl
  • Add support for SciMLOperators in downstream packages.
  • Replaces subtypes of *DiffEqOperators to AbstractSciMLOperators. Do NOT remove support for DiffEqOperators. Order below:
  • LinearSolve.jl - make preconditioners AbstractSciMLOperators
  • Sundials.jl
  • DiffEqOperators.jl
  • SparseDiffTools.jl - make AD operators AbstractSciMLOperators
  • DiffEqSensitivity.jl - use SparseDiffTools AD operators in SteadyStateAdjoint. Use adjoint(A) if islinear(A) = true, has_adjoint(A) = true.
  • OrdinaryDiffEq.jl - form WOperator just by composing AbstractSciMLOperator: W = 1/gamma * M - J or the opp
  • StochasticDiffEq.jl
  • Once SciMLOperators are supported everywhere, remove DiffEqOperators starting from downstream packages to upstream packages.
  • Finally deprecate AbstractDiffEqOperator family in SciMLBase.jl
  • remove DiffEqOperators from docs

EDITS -

TODOs

SciMLBase.jl

deprecate all diffeqoperator subtypes

LinearSolve.jl

  • update preconditioner docs
  • deprecations:
const ComposePreconditioner = SciMLOperators.ComposedOperator
@deprecate ComposePreconditioner SciMLOperators.ComposedOperator

const InvPreconditioner = SciMLOperators.InvertedOperator
@deprecate InvPreconditioner SciMLOperators.InvertedOperator
  • in defaultalg selection, as well as in init_cacheval, OperatorAssumptions{nothing} behaves exactly like OperatorAssumptions{true}. So let's remove the option to put in nothing.
  • in src/common.jl, set_A also modify cache.OperatorAssumptions.
  • in src/default.jl, remove method defaultalg(A::Diagonal, b, ::OperatorAssumptions{false}) because diagonal matrices are always square
  • In src/factorization.jl, DiagonalFactorization should ideally preinvert the D.diag in init_cacheval, and then mul! in SciMLBase.solve. Create a separate PR
  • replace IterativeSolvers: Identity in OrdinaryDiffEq with IdentityOperator.

DiffEqBase.jl

In test/basic_operator_tests.jl, test/affine_operator_tests.jl, there are tests for DiffEqOperators. Let's leave that as it is till DiffEqOps are remove from downstream packages.

SparseDiffTools.jl

  • Add OrdinaryDiffEq.jl - InterfaceII to downstream tests. Same for SciMLSensitivity.jl when we get done with that.
  • Add JacVec ODE tests
  • autodiff shouldn't be boolean logic but multi-valued logic through types choices, like AutoZygote(). change the autodiff choices to be non-boolean logic and Zygote an extension package in order to release.
  • release v2 Release v2 JuliaDiff/SparseDiffTools.jl#230
# JacVec OrdinaryDiffEq itnegration test

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
u0 = [1.0; 0.0; 0.0]
tspan = (0.0, 100.0)

ff1 = ODEFunction(lorenz, jac_prototype = JacVec(lorenz, u0))
ff2 = ODEFunction(lorenz, jac_prototype = JacVec(lorenz, u0, autodiff=false))

for ff in [ff1, ff2]
    prob = ODEProblem(ff, u0, tspan)
    @test solve(prob, TRBDF2()).retcode == :Success
    @test solve(prob, TRBDF2(linsolve = KrylovJL_GMRES())).retcode == :Success
    @test solve(prob, Exprb32()).retcode == :Success
    @test solve(prob, Rosenbrock23()).retcode == :Success
    @test solve(prob, Rosenbrock23(linsolve = KrylovJL_GMRES())).retcode == :Success
end
@vpuri3
Copy link
Member Author

vpuri3 commented Jan 30, 2023

CC: @ChrisRackauckas, @xtalax

@xtalax
Copy link
Member

xtalax commented Jan 31, 2023

Ready to help, but not sure how helpful I can be towards operator translation as I don't fully understand their requirements

@vpuri3
Copy link
Member Author

vpuri3 commented Feb 19, 2023

@ChrisRackauckas LMK when you register sparsedifftools v2.0, and I'll start with SciMLSensitivity, OrdinaryDiffEq

@ChrisRackauckas
Copy link
Member

It should be hopefully alter this week. It needs JuliaDiff/SparseDiffTools.jl#215 to be completed first, but I can help with the bits there. Note that the ArrayInterface overhaul "comes first" and may clog up the pipeline this week, hopefully that moves fast.

@vpuri3
Copy link
Member Author

vpuri3 commented Mar 22, 2023

@gaurav-arya do you wanna take a stab at OrdinaryDiffeq

@gaurav-arya
Copy link
Member

yes, happy to since I've already looked at a lot of the operator code there. Would need a merge of #143 though

@vpuri3
Copy link
Member Author

vpuri3 commented Mar 23, 2023

there are many things to do in OrdinaryDiffEq. while the kwargs PR is in review can you do a small PR in OrdinaryDiffEq to support Sparsedifftools v2? that's blocking the scimlsensitivity pr.

@gaurav-arya
Copy link
Member

Sounds good, I'll get to it Friday/Saturday

@ChrisRackauckas
Copy link
Member

I had to pull back on OrdinaryDiffEq.jl for now because it needs to update update_coefficients! for the arguments change. And Sundials.jl is getting a test failure due to the change of JacVec (it doesn't depend on SparseDiffTools but does use JacVec in a test).

@gaurav-arya
Copy link
Member

gaurav-arya commented Mar 23, 2023

For the arguments change, are you referring to #143 or something else? Because #143 should be totally backwards compatible -- it gives SciMLOperators the flexibility to accept keyword arguments, but only in special situations when they expect to receive them from the caller.

@ChrisRackauckas
Copy link
Member

No it's a different one, the one associated with the SparseDiffTools v2 update.

@vpuri3
Copy link
Member Author

vpuri3 commented Mar 23, 2023

@ChrisRackauckas , is there an issue/PR for that?

@vpuri3
Copy link
Member Author

vpuri3 commented Mar 23, 2023

Thanks for sharing. I see that both are using SparseDiffTools v2. A quick fix would be to restrict compatibility to 1.x. We can then have PRs to update to v2.

@ChrisRackauckas
Copy link
Member

Yup so I rolled it back this morning and it'll need some real updates for compatability.

@vpuri3
Copy link
Member Author

vpuri3 commented Mar 23, 2023

Thanks. So @gaurav-arya, if you have time this weekend, can you update OrdinaryDiffEq to work with sparsedifftools v2?

@xtalax
Copy link
Member

xtalax commented Jul 19, 2023

Man that checklist is looking like it's got momentum, good work so far.

@xtalax
Copy link
Member

xtalax commented Jul 19, 2023

Tracking DEO in the docs: SciML/DiffEqDocs.jl#678

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants