make AD operators scimloperators #212

vpuri3 · 2023-02-04T20:38:50Z

codecov-commenter · 2023-02-06T21:04:13Z

Codecov Report

Base: 0.22% // Head: 82.92% // Increases project coverage by +82.70% 🎉

Coverage data is based on head (f322ec7) compared to base (a91d1da).
Patch coverage: 84.84% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #212       +/-   ##
===========================================
+ Coverage    0.22%   82.92%   +82.70%     
===========================================
  Files          15       15               
  Lines         887      949       +62     
===========================================
+ Hits            2      787      +785     
+ Misses        885      162      -723

Impacted Files	Coverage Δ
src/SparseDiffTools.jl	`100.00% <ø> (+60.00%)`	⬆️
src/differentiation/vecjac_products.jl	`62.74% <73.07%> (+62.74%)`	⬆️
src/differentiation/jaches_products_zygote.jl	`93.18% <83.33%> (+93.18%)`	⬆️
src/differentiation/jaches_products.jl	`95.62% <90.74%> (+95.62%)`	⬆️
src/differentiation/vecjac_products_zygote.jl	`57.14% <100.00%> (+57.14%)`	⬆️
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

vpuri3 · 2023-02-06T21:40:32Z

@ChrisRackauckas @YingboMa can you take a look? Maybe suggest some tests for VecJacProd operator

vpuri3 · 2023-02-06T23:22:03Z

TODO - add 5 arg mul! in test, and add tests for VecJacProd.

vpuri3 · 2023-02-07T18:11:28Z

TODO - write 5-arg mul! tests after SciML/SciMLOperators.jl#150

vpuri3 · 2023-02-07T19:57:27Z

@ChrisRackauckas this should be good. Please take a look

test/runtests.jl

vpuri3 · 2023-02-08T19:13:54Z

@ChrisRackauckas

ChrisRackauckas · 2023-02-08T21:28:00Z

That looks fine in general, but why is it not just replacing the old ones? Delete the old ones and use their name?

vpuri3 · 2023-02-08T21:34:55Z

OrdinaryDiffEq uses JecVac in src/derivative_utils.jl. So deleting the old ones would break stuff there. Once we're done updating OrdinaryDiffEq, we can delete the old ones

ChrisRackauckas · 2023-02-08T21:38:42Z

This should be a drop-in replacement though?

vpuri3 · 2023-02-08T21:45:33Z

No. ODE.jl uses the type JacVec. Eg

if W.J !== nothing && !(W.J isa SparseDiffTools.JacVec)

I don't have the type JacVec anymore. My functions JacVecProd, etc form a FunctionOperator

ChrisRackauckas · 2023-02-08T21:47:45Z

Then we can make this a breaking release v2, with this change and adding the ArrayInterface packages from the README as extension packages.

src/differentiation/jaches_products.jl

Project.toml

ChrisRackauckas · 2023-02-10T16:06:35Z

test/test_jaches_products.jl

+#=
+ff1 = ODEFunction(lorenz, jac_prototype = JacVec{Float64}(lorenz, u0))
+ff2 = ODEFunction(lorenz, jac_prototype = JacVec{Float64}(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
+=#
+
+# HesVec


OrdinaryDiffEq fails to precompile with this version of SparseDiffTools because of the changes we have discussed above. So I'm going to remove these tests and put them back in once we make the requisite changes in OrdianryDiffEq (in OrdinaryDiffEq.jl/src/derivative_utils.jl). Making a note of that here: SciML/SciMLOperators.jl#142

src/differentiation/jaches_products.jl

ChrisRackauckas · 2023-02-11T14:05:42Z

src/differentiation/vecjac_products.jl

+    isinplace  = static_hasmethod(f, typeof((u, p, t)))
+    outofplace = static_hasmethod(f, typeof((u, u, p, t)))
+
+    if !(isinplace) & !(outofplace)
+        error("$f must have signature f(u, p, t), or f(du, u, p, t)")
+    end
+
+    L = RevModeAutoDiffVecProd(f, u, cache, vecprod, vecprod!; autodiff = autodiff,
+                               isinplace = isinplace, outofplace = outofplace)


The other function operators need this treatment too

ChrisRackauckas · 2023-02-11T14:08:07Z

src/differentiation/vecjac_products.jl

+    vecprod  = autodiff ? auto_vecjac  : num_vecjac
+    vecprod! = autodiff ? auto_vecjac! : num_vecjac!


This should also have a choice for Zygote. For stability, JvFormZygote() and such? Use the same one the Jv operators.

I'll do this as a follow up, it should be included in the breaking update.

vpuri3 · 2023-02-18T21:40:36Z

currently, JacVec, HesVec, HesVecGrad do not use p, t. They accept functions like f(x), f(y, x).

function (L::FwdModeAutoDiffVecProd)(v, p, t)
    L.vecprod(L.f, L.u, v)
end

As compared to VecJac that accepts f(u, p, t), f(du, u, p, t)

# Interpret the call as df/du' * u
function (L::RevModeAutoDiffVecProd)(v, p, t)
    L.vecprod(_u -> L.f(_u, p, t), L.u, v)
end

Do we want to enforce the same (u,p,t) signature in the former?

ChrisRackauckas · 2023-02-19T04:16:34Z

No, these don't have p and t.

vpuri3 · 2023-02-19T12:05:30Z

ok, then this is complete. needs to rerun ci once array interface stuff is cleared out

…nd bump

src/differentiation/jaches_products.jl

ChrisRackauckas · 2023-02-19T13:18:13Z

src/differentiation/vecjac_products_zygote.jl

@@ -7,3 +7,6 @@ function auto_vecjac(f, x, v)
    vv, back = Zygote.pullback(f, x)
    return vec(back(reshape(v, size(vv)))[1])
 end
+
+const ZygoteVecJac = VecJac


what's this for?

ChrisRackauckas · 2023-02-19T13:20:50Z

Just two small questions are left. After this merges, then we should change the autodiff choices to be non-boolean logic and Zygote an extension package in order to release.

vpuri3 · 2023-02-19T17:34:50Z

@ChrisRackauckas about the zygote stuff...

You mentioned above that we need Zygote operators for stability. I wrote functions ZygoteVecJac and ZygoteHesVec that use num/auto_vecjac(!), num/autoback_hesvec(!).

The difference between VecJac and ZygoteVecJac is that the former defaults to autodiff = false and the later true. That way VecJac won't error with default kwargs if zygote isn't loaded. Since VecJac uses auto_vecjac(!) when autodiff = true, I included a warning in there to make sure Zygote is loaded:

    if autodiff 
        @assert isdefined(SparseDiffTools, :auto_vecjac) "Please load Zygote with `using Zygote`, or `import Zygote` to use VecJac with `autodiff = true`."
    end

ChrisRackauckas · 2023-02-19T20:25:49Z

You mentioned above that we need Zygote operators for stability. I wrote functions ZygoteVecJac and ZygoteHesVec that use num/auto_vecjac(!), num/autoback_hesvec(!).

No, I meant that autodiff shouldn't be boolean logic but multi-valued logic through types choices, like AutoZygote().

ChrisRackauckas · 2023-02-19T20:26:51Z

We can follow up with the extension package stuff.

vpuri3 changed the title ~~[WIP] make AD operators scimloperators~~ make AD operators scimloperators Feb 7, 2023

ChrisRackauckas reviewed Feb 8, 2023

View reviewed changes

test/runtests.jl Outdated Show resolved Hide resolved

ChrisRackauckas reviewed Feb 9, 2023

View reviewed changes

src/differentiation/jaches_products.jl Outdated Show resolved Hide resolved

ChrisRackauckas reviewed Feb 10, 2023

View reviewed changes

Project.toml Show resolved Hide resolved

ChrisRackauckas reviewed Feb 10, 2023

View reviewed changes

vpuri3 mentioned this pull request Feb 10, 2023

no deo bruss (was wrong anyway) SciML/DiffEqProblemLibrary.jl#103

Merged

ChrisRackauckas reviewed Feb 11, 2023

View reviewed changes

src/differentiation/jaches_products.jl Show resolved Hide resolved

ChrisRackauckas reviewed Feb 11, 2023

View reviewed changes

vpuri3 added 7 commits February 19, 2023 08:07

ignore vim files

5f0714e

add scimlops

3fefab1

JVP WIP

1691faa

VJP stuff copy pasted from JuliaDiff#189

c5ef8ae

comments

5d41abf

scimloperator compat: wait for SciML/SciMLOperators.jl#147 to merge a…

0de693c

…nd bump

make all these structs FunctionOperators

e1e7c95

vpuri3 added 17 commits February 19, 2023 08:07

tests

1fad88b

typo

2e90ffc

cleanup

78eb0bf

vecjacprod tests

fbda265

5 arg mul<bang> tests

5aa1f0d

comments, 5-arg mul<bang>

4b0fcce

uncomment tests

1796880

v2

e1d8be1

redefine JacVec, HesVec, HesVecGrad, VecJac

84542bb

tests

9375983

no inplace tests for rev mode

75974ed

fix typo

efb05a5

dont need multiple update_coeff definitinos

edac7af

rm ODE JacVec tests

866daa1

tricks compat

ec9e0f4

iip,oop

a8f374b

zygote

ce0febe

ChrisRackauckas force-pushed the scimlops branch from 15b01e5 to ce0febe Compare February 19, 2023 13:07

remove other arrayinterfaces in tests

df9b45a

ChrisRackauckas reviewed Feb 19, 2023

View reviewed changes

src/differentiation/jaches_products.jl Outdated Show resolved Hide resolved

ChrisRackauckas reviewed Feb 19, 2023

View reviewed changes

src/differentiation/jaches_products.jl Show resolved Hide resolved

ChrisRackauckas reviewed Feb 19, 2023

View reviewed changes

vpuri3 added 2 commits February 19, 2023 11:42

make structs immutable

4c760e8

Zygote

f322ec7

ChrisRackauckas merged commit d79a00f into JuliaDiff:master Feb 19, 2023

vpuri3 deleted the scimlops branch February 19, 2023 21:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make AD operators scimloperators #212

make AD operators scimloperators #212

vpuri3 commented Feb 4, 2023 •

edited

Loading

codecov-commenter commented Feb 6, 2023 •

edited

Loading

vpuri3 commented Feb 6, 2023

vpuri3 commented Feb 6, 2023

vpuri3 commented Feb 7, 2023

vpuri3 commented Feb 7, 2023

vpuri3 commented Feb 8, 2023

ChrisRackauckas commented Feb 8, 2023

vpuri3 commented Feb 8, 2023 •

edited

Loading

ChrisRackauckas commented Feb 8, 2023

vpuri3 commented Feb 8, 2023 •

edited

Loading

ChrisRackauckas commented Feb 8, 2023

ChrisRackauckas Feb 10, 2023

vpuri3 Feb 10, 2023

ChrisRackauckas Feb 11, 2023

ChrisRackauckas Feb 11, 2023

ChrisRackauckas Feb 19, 2023

vpuri3 commented Feb 18, 2023 •

edited

Loading

ChrisRackauckas commented Feb 19, 2023

vpuri3 commented Feb 19, 2023

ChrisRackauckas Feb 19, 2023

ChrisRackauckas commented Feb 19, 2023

vpuri3 commented Feb 19, 2023

ChrisRackauckas commented Feb 19, 2023

ChrisRackauckas commented Feb 19, 2023

		vecprod = autodiff ? auto_vecjac : num_vecjac
		vecprod! = autodiff ? auto_vecjac! : num_vecjac!

make AD operators scimloperators #212

make AD operators scimloperators #212

Conversation

vpuri3 commented Feb 4, 2023 • edited Loading

codecov-commenter commented Feb 6, 2023 • edited Loading

Codecov Report

vpuri3 commented Feb 6, 2023

vpuri3 commented Feb 6, 2023

vpuri3 commented Feb 7, 2023

vpuri3 commented Feb 7, 2023

vpuri3 commented Feb 8, 2023

ChrisRackauckas commented Feb 8, 2023

vpuri3 commented Feb 8, 2023 • edited Loading

ChrisRackauckas commented Feb 8, 2023

vpuri3 commented Feb 8, 2023 • edited Loading

ChrisRackauckas commented Feb 8, 2023

ChrisRackauckas Feb 10, 2023

Choose a reason for hiding this comment

vpuri3 Feb 10, 2023

Choose a reason for hiding this comment

ChrisRackauckas Feb 11, 2023

Choose a reason for hiding this comment

ChrisRackauckas Feb 11, 2023

Choose a reason for hiding this comment

ChrisRackauckas Feb 19, 2023

Choose a reason for hiding this comment

vpuri3 commented Feb 18, 2023 • edited Loading

ChrisRackauckas commented Feb 19, 2023

vpuri3 commented Feb 19, 2023

ChrisRackauckas Feb 19, 2023

Choose a reason for hiding this comment

ChrisRackauckas commented Feb 19, 2023

vpuri3 commented Feb 19, 2023

ChrisRackauckas commented Feb 19, 2023

ChrisRackauckas commented Feb 19, 2023

vpuri3 commented Feb 4, 2023 •

edited

Loading

codecov-commenter commented Feb 6, 2023 •

edited

Loading

vpuri3 commented Feb 8, 2023 •

edited

Loading

vpuri3 commented Feb 8, 2023 •

edited

Loading

vpuri3 commented Feb 18, 2023 •

edited

Loading