Skip to content

Commit d65bfdd

Browse files
committed
Revert "fix it and remove global check"
This reverts commit 17b4411.
1 parent 17b4411 commit d65bfdd

File tree

6 files changed

+69
-34
lines changed

6 files changed

+69
-34
lines changed

docs/src/index.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ For information about ChainRules, including how to write rules, refer to the gen
99
[![](https://img.shields.io/badge/docs-main-blue.svg)](https://JuliaDiff.github.io/ChainRulesCore.jl/dev)
1010
[![](https://img.shields.io/badge/docs-stable-blue.svg)](https://JuliaDiff.github.io/ChainRulesCore.jl/stable)
1111

12+
## Testing Method Table Sensibility
13+
A basic feature of ChainRulesTestUtils is its ability to check that the method tables for `rrule` and `frule` remain sensible.
14+
This searches the method tables for methods that should not exist and when it fails tells you where they were defined.
15+
By calling [`test_method_tables`](@ref) ChainRulesTestUtils will check for things such as having attracted a rule to `DataType` rather than attaching it to a constructor.
16+
Basically all packages using ChainRulesTestUtils can use [`test_method_tables`](@ref), as it is independent of what rules you have written.
17+
1218
## Canonical example of testing frule and rrule
1319

1420
Let's suppose a custom transformation has been defined

src/ChainRulesTestUtils.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ include("iterator.jl")
2828

2929
include("output_control.jl")
3030
include("check_result.jl")
31-
include("method_checks.jl")
3231

3332
include("rule_config.jl")
3433
include("finite_difference_calls.jl")
3534
include("testers.jl")
3635

36+
include("global_checks.jl")
3737
end # module

src/method_checks.jl renamed to src/global_checks.jl

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ function test_method_signature(::typeof(rrule), method::Method)
2626
function_type = if method.sig <: Tuple{Any, RuleConfig, Type, Vararg}
2727
_parameters(method.sig)[3]
2828
elseif method.sig <: Tuple{Any, Type, Vararg}
29-
_parameters(method.sig)[2]
30-
else
29+
_parameters(method.sig)else
3130
nothing
3231
end
3332

@@ -54,3 +53,29 @@ function test_method_signature(::typeof(frule), method::Method)
5453
)
5554
end
5655
end
56+
57+
"""
58+
test_method_tables()
59+
60+
Checks that the method tables for `rrule` and `frule` are sensible.
61+
This in future may carry out a number of checks, but presently just checks to make sure that
62+
no rules have been added to the very general `DataType`, `Union` or `UnionAll` types,
63+
which is easy to do accidentally when writing rules for constructors.
64+
It happens if you write e.g. `rrule(::typeof(Foo), x)` rather than `rrule(::Type{<:Foo}, x)`.
65+
This would then actually define `rrule(::DataType, x)`. (or `UnionAll` if `Foo`
66+
was parametric, or `Union` if `Foo` was a type alias for a `Union`)
67+
"""
68+
function test_method_tables()
69+
@testset "Sensible Constructors" begin
70+
# if someone wrote e.g. `rrule(::typeof(Foo), x)` rather than
71+
# `rrule(::Type{<:Foo}, x)` then that would actually define `rrule(::DataType, x)`
72+
# which would be bad. This test checks for that and fails if such a method exists.
73+
for method in methods(rrule)
74+
test_method_signature(method)
75+
end
76+
# frule
77+
for method in methods(frule)
78+
test_method_signature(frule, method)
79+
end
80+
end
81+
end

test/global_checks.jl

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
struct BadConstructor
2+
x
3+
end
4+
5+
if VERSION >= v"1.3"
6+
@testset "global_checks.jl" begin
7+
test_method_tables()
8+
ChainRulesCore.rrule(::typeof(BadConstructor), x) = nothing
9+
@test fails(test_method_tables)
10+
Base.delete_method(Base.which(rrule, (DataType, Any)))
11+
test_method_tables() # make sure delete worked
12+
13+
ChainRulesCore.rrule(::RuleConfig, ::typeof(BadConstructor), x) = nothing
14+
@test fails(test_method_tables)
15+
Base.delete_method(Base.which(rrule, (RuleConfig, DataType, Any)))
16+
test_method_tables() # make sure delete worked
17+
18+
19+
20+
ChainRulesCore.frule(::Any, ::typeof(BadConstructor), x) = nothing
21+
@test fails(test_method_tables)
22+
Base.delete_method(Base.which(frule, (Any, DataType, Any)))
23+
test_method_tables() # make sure delete worked
24+
25+
ChainRulesCore.frule(::RuleConfig, ::Any, ::typeof(BadConstructor), x) = nothing
26+
@test fails(test_method_tables)
27+
Base.delete_method(Base.which(frule, (RuleConfig, Any, DataType, Any)))
28+
test_method_tables() # make sure delete worked
29+
end
30+
else # pre 1.3, so no `delete_method` so just test happy path
31+
@testset "global_checks.jl" begin
32+
test_method_tables()
33+
end
34+
end

test/method_checks.jl

Lines changed: 0 additions & 30 deletions
This file was deleted.

test/runtests.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@ ChainRulesTestUtils.TEST_INFERRED[] = true
1717
include("data_generation.jl")
1818
include("rand_tangent.jl")
1919

20-
include("method_checks.jl")
20+
include("global_checks.jl")
2121
end

0 commit comments

Comments
 (0)