JuliaDiff / SparseDiffTools.jl

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

Zygote ext #218

Closed vpuri3 closed 1 year ago

vpuri3 commented 1 year ago

part of https://github.com/JuliaDiff/SparseDiffTools.jl/issues/215

ref https://pkgdocs.julialang.org/dev/creating-packages/#Conditional-loading-of-code-in-packages-(Extensions)

vpuri3 commented 1 year ago

running CI with dummy change since tests were failing locally.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 10.52% and project coverage change: +0.10 :tada:

Comparison is base (c2bbf98) 0.21% compared to head (21a9a1e) 0.31%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #218 +/- ## ========================================= + Coverage 0.21% 0.31% +0.10% ========================================= Files 15 14 -1 Lines 936 940 +4 ========================================= + Hits 2 3 +1 - Misses 934 937 +3 ``` | [Impacted Files](https://codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/218?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://codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff#diff-ZXh0L1NwYXJzZURpZmZUb29sc1p5Z290ZS5qbA==) | `0.00% <0.00%> (ø)` | | | [src/SparseDiffTools.jl](https://codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff#diff-c3JjL1NwYXJzZURpZmZUb29scy5qbA==) | `37.50% <50.00%> (-2.50%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff)

: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 looks like SparseDiffTools needs more comprehensive testing. I am getting arrayinerface errors with 1.9 on my personal machine that are not captured on CI.

vpuri3 commented 1 year ago

Looks like package extension don't honor export statements. Rather, it is just

julia> versioninfo()
Julia Version 1.9.0-rc1
Commit 3b2e0d8fbc1 (2023-03-07 07:51 UTC)

(a) pkg> st                                                                          
Status `~/.julia/dev/SparseDiffTools/a/Project.toml`
  [47a9eef4] SparseDiffTools v2.0.0 `..`
  [e88e6eb3] Zygote v0.6.56                                                          

julia> using SparseDiffTools, Zygote  
[ Info: Precompiling SparseDiffToolsZygote [25f82377-10ae-5fb8-bc0d-27fd31de4285]

julia> m = Base.get_extension(SparseDiffTools, :SparseDiffToolsZygote)
SparseDiffToolsZygote 

julia> m.ZygoteVecJac
ZygoteVecJac (generic function with 1 method)

julia> ZygoteVecJac
ERROR: UndefVarError: `ZygoteVecJac` not defined

julia> SparseDiffTools.ZygoteVecJac
ERROR: UndefVarError: `ZygoteVecJac` not defined
Stacktrace:
 [1] getproperty(x::Module, f::Symbol)

So I think we should define and export these symbols in the main package, and write method definitions in the extension.

vpuri3 commented 1 year ago

@ChrisRackauckas can you merge https://github.com/JuliaDiff/SparseDiffTools.jl/pull/220 so we can test for 1x9 here?

vpuri3 commented 1 year ago

rerunning CI

vpuri3 commented 1 year ago

@ChrisRackauckas good to go