DiffRules.jl
DiffRules.jl copied to clipboard
Revert "comment out trinary rules"
This reverts #60, thus un-reverts #54.
Wants to be run with #61 and https://github.com/JuliaDiff/ForwardDiff.jl/pull/530 before merging.
Codecov Report
Patch coverage: 100.00% and project coverage change: +0.03 :tada:
Comparison is base (
2001650) 97.86% compared to head (6a7aeef) 97.89%.
Additional details and impacted files
@@ Coverage Diff @@
## master #62 +/- ##
==========================================
+ Coverage 97.86% 97.89% +0.03%
==========================================
Files 3 3
Lines 187 190 +3
==========================================
+ Hits 183 186 +3
Misses 4 4
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/rules.jl | 100.00% <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.
Thanks for looking. I thought so but indeed we should re-check.
ForwardDiff v0.10.18 and below have the error if there is a trinary rule. This is the line upper-bounding their DiffRules in the registry:
https://github.com/JuliaRegistries/General/blob/master/F/ForwardDiff/Compat.toml#L50-L51
The current version is ForwardDiff v0.10.19 which accepts only DiffRules 1.2.1 and up:
https://github.com/JuliaDiff/ForwardDiff.jl/blob/v0.10.19/Project.toml
This is reflected here in the registry:
https://github.com/JuliaRegistries/General/blob/master/F/ForwardDiff/Compat.toml#L22-L23
lines added by the robot https://github.com/JuliaRegistries/General/pull/41858 . So future releases should also work smoothly I believe, not touch old bounds.
So I think that implies this is safe.
@mcabbott I updated the PR and fixed some tests, do you have any comments? Otherwise I think this PR can be merged.