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

add tag kwarg to JacVec, HesVec, HesVecGrad #241

Merged
merged 4 commits into from
May 15, 2023

Conversation

vpuri3
Copy link
Contributor

@vpuri3 vpuri3 commented May 11, 2023

This will fix errors in SciML/OrdinaryDiffEq.jl#1917 by allowing OrdinaryDiffEq to pass the right ForwardDiff.tag to JacVec.

@codecov
Copy link

codecov bot commented May 11, 2023

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
@@           Coverage Diff           @@
##           master     #241   +/-   ##
=======================================
  Coverage   84.76%   84.76%           
=======================================
  Files          14       14           
  Lines         952      952           
=======================================
  Hits          807      807           
  Misses        145      145           
Impacted Files Coverage Δ
ext/SparseDiffToolsZygote.jl 100.00% <100.00%> (ø)
src/differentiation/jaches_products.jl 95.37% <100.00%> (ø)

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

@vpuri3
Copy link
Contributor Author

vpuri3 commented May 11, 2023

@ChrisRackauckas

vpuri3 added a commit to gaurav-arya/OrdinaryDiffEq.jl that referenced this pull request May 12, 2023
@vpuri3
Copy link
Contributor Author

vpuri3 commented May 12, 2023

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

@ChrisRackauckas
Copy link
Member

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

@vpuri3
Copy link
Contributor Author

vpuri3 commented May 12, 2023

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
Copy link
Contributor Author

vpuri3 commented May 12, 2023

@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 SciML/SciMLOperators.jl#142

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 vpuri3 mentioned this pull request May 12, 2023
@ChrisRackauckas ChrisRackauckas merged commit 7cb7918 into JuliaDiff:master May 15, 2023
@vpuri3 vpuri3 deleted the tag branch May 15, 2023 14:00
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

Successfully merging this pull request may close these issues.

2 participants