Closed mohamed82008 closed 2 years ago
This addresses the motivation behind #39.
Merging #49 (6207218) into master (9a3b564) will increase coverage by
0.14%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #49 +/- ##
==========================================
+ Coverage 83.22% 83.36% +0.14%
==========================================
Files 5 6 +1
Lines 465 475 +10
==========================================
+ Hits 387 396 +9
- Misses 78 79 +1
Impacted Files | Coverage Δ | |
---|---|---|
src/AbstractDifferentiation.jl | 78.41% <100.00%> (-0.10%) |
:arrow_down: |
src/ruleconfig.jl | 100.00% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 9a3b564...6207218. Read the comment docs.
Seems that the Julia 1.0 tests are failing because ZygoteRuleConfig is only available in a recent version of Zygote which is not compatible with Julia 1.0
@odow is JuMP likely to drop Julia 1.0 support any time soon?
RuleConfig support is now available only in Julia 1.6+. I will go ahead and merge this then if 1.0 support is still required for RuleConfig, we can open another issue/PR discussing/implementing that.
This PR implements RuleConfig support via rrule_via_ad. I tested Zygote and tests pass. I tried Yota but tests didn't pass for some reason so I will open another PR. This is a very minimal PR to get things working but I didn't do the excellent benchmarks @sethaxen does in these kinds of PRs. I think optimisations can be added as a later PR.