JuliaDiff / SparseDiffTools.jl

Fast jacobian computation through sparsity exploitation and matrix coloring
MIT License
237 stars 41 forks source link

add `tag` kwarg to JacVec, HesVec, HesVecGrad #241

Closed vpuri3 closed 1 year ago

vpuri3 commented 1 year ago

This will fix errors in https://github.com/SciML/OrdinaryDiffEq.jl/pull/1917 by allowing OrdinaryDiffEq to pass the right ForwardDiff.tag to JacVec.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (c831947) 84.76% compared to head (e77e83f) 84.76%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #241 +/- ## ======================================= Coverage 84.76% 84.76% ======================================= Files 14 14 Lines 952 952 ======================================= Hits 807 807 Misses 145 145 ``` | [Impacted Files](https://app.codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/241?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff) | Coverage Δ | | |---|---|---| | [ext/SparseDiffToolsZygote.jl](https://app.codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/241?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff#diff-ZXh0L1NwYXJzZURpZmZUb29sc1p5Z290ZS5qbA==) | `100.00% <100.00%> (ø)` | | | [src/differentiation/jaches\_products.jl](https://app.codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/241?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff#diff-c3JjL2RpZmZlcmVudGlhdGlvbi9qYWNoZXNfcHJvZHVjdHMuamw=) | `95.37% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

vpuri3 commented 1 year ago

@ChrisRackauckas

vpuri3 commented 1 year ago

@Vaibhavdixit02 , @YingboMa can you review and merge this? It's blocking other PRs

ChrisRackauckas commented 1 year ago

This needs a test so it doesn't regress again.

vpuri3 commented 1 year ago

added some tests here. After the ODE PR is merged, we can add ODE tests in this repo, as well as downstream tests to OrdinaryDiffEq in SparseDiffTools.

vpuri3 commented 1 year ago

@ChrisRackauckas take a look. more comprehensive tests can be added only after ODE.jl supports sparsediff v2. Here's my todo list for sparsedifftools from https://github.com/SciML/SciMLOperators.jl/issues/142

SparseDiffTools.jl

  • [ ] Add OrdinaryDiffEq.jl - InterfaceII to downstream tests. Same for SciMLSensitivity.jl when we get done with that.
  • [ ] Add JacVec ODE tests
  • [x] 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.
  • [x] 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