-
Notifications
You must be signed in to change notification settings - Fork 15
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
test arbitrary functions with f/rrule
-like API
#166
Changes from 18 commits
9d461d6
a95965e
911f75d
c87e7ae
7f84bce
9d9f730
3428e96
11f57d6
c5e5e75
58170be
ffe7624
b2d0771
84bd5a1
fadde23
9ab02fd
6198bcf
3213d31
797fe44
26d769c
3067669
9672320
6cabef0
71a08c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
# API Documentation | ||
|
||
```@autodocs | ||
Modules = [ChainRulesTestUtils] | ||
Private = false | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,8 +30,8 @@ for (T1, T2) in ((AbstractThunk, Any), (AbstractThunk, AbstractThunk), (Any, Abs | |
end | ||
end | ||
|
||
test_approx(::ZeroTangent, x, msg=""; kwargs...) = test_approx(zero(x), x, msg; kwargs...) | ||
test_approx(x, ::ZeroTangent, msg=""; kwargs...) = test_approx(x, zero(x), msg; kwargs...) | ||
test_approx(::AbstractZero, x, msg=""; kwargs...) = test_approx(zero(x), x, msg; kwargs...) | ||
test_approx(x, ::AbstractZero, msg=""; kwargs...) = test_approx(x, zero(x), msg; kwargs...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason I've changed this is somewhat convoluted and the dependency goes the wrong way. But hear me out: When writing a helper Alternatives:
All of the above aside, I am leaning towards the current state where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than allowing it in general between
Doing this in Zygote's tests though doesn't seem that awful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is exactly the
I thought using Zygote
test_rrule(myfunc, args...; rrule_f=rrule_via_ad) If we put the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm Ok. lets do it. aside: the new way to do this is going to be test_rrule(ZygoteRuleConfig, myfunc, args...; rrule_f=rrule_via_ad) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, thanks, it's already changed in the code |
||
test_approx(x::ZeroTangent, y::ZeroTangent, msg=""; kwargs...) = @test true | ||
|
||
# remove once https://github.com/JuliaDiff/ChainRulesTestUtils.jl/issues/113 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# For testing this config re-dispatches Xrule_via_ad to Xrule without config argument | ||
struct ADviaRuleConfig <: RuleConfig{Union{HasReverseMode, HasForwardsMode}} end | ||
|
||
function ChainRulesCore.frule_via_ad(config::ADviaRuleConfig, ȧrgs, f, args...; kws...) | ||
ret = frule(config, ȧrgs, f, args...; kws...) | ||
# we don't support actually doing AD: the rule has to exist. lets give helpfulish error | ||
ret === nothing && throw(MethodError(frule, (ȧrgs, f, args...))) | ||
return ret | ||
end | ||
|
||
function ChainRulesCore.rrule_via_ad(config::ADviaRuleConfig, f, args...; kws...) | ||
ret = rrule(config, f, args...; kws...) | ||
# we don't support actually doing AD: the rule has to exist. lets give helpfulish error | ||
ret === nothing && throw(MethodError(rrule, (f, args...))) | ||
return ret | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be worth hard-coding a link to the ChainRulesCore docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.