Add ZeroTangent vs array of arrays tests

[alyst]

This fixes the test_approx() method call errors in DistributionsAD.jl unit tests that I came across while working on TuringLang/DistributionsAD.jl#227.

[codecov-commenter]

Codecov Report

Merging #257 (b06c25a) into main (5b9ec32) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #257      +/-   ##
==========================================
+ Coverage   93.41%   93.45%   +0.03%     
==========================================
  Files          12       12              
  Lines         334      336       +2     
==========================================
+ Hits          312      314       +2     
  Misses         22       22              

Impacted Files	Coverage Δ
src/check_result.jl	90.27% <100.00%> (+0.27%)	⬆️

---

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 5b9ec32...b06c25a. Read the comment docs.

[mzgubic]

Thanks for the PR, looks good to me!

I assume you mean this one https://github.com/TuringLang/DistributionsAD.jl/pull/227?

[devmotion]

Maybe a recursive definition such as

Suggested change

	test_approx(::AbstractZero, x::AbstractArray{<:AbstractArray}, msg=""; kwargs...) = test_approx(map(zero, x), x, msg; kwargs...)
	test_approx(x::AbstractZero, y::AbstractArray{<:AbstractArray}, msg=""; kwargs...) = all(yi -> test_approx(x, yi, msg; kwargs...), y)

would be easier as it would pick up test_approx definitions for the elements automatically, without having to redefine the array of arrays method?

[mzgubic]

The issue is that the test_approx does not return a boolean, but rather does the @test. So the suggestion above results in something like

julia> test_approx(ZeroTangent(), [[0, 0.0], [0.0, 0.1], [[0.0, 0.0], [0.0, 0.0]]])
ERROR: TypeError: non-boolean (Test.Pass) used in boolean context
Stacktrace:
 [1] _all(f::ChainRulesTestUtils.var"#113#114"{Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}, ZeroTangent, String}, itr::Vector{Vector}, #unused#::Colon)
   @ Base ./reduce.jl:1161
 [2] all(f::Function, a::Vector{Vector}; dims::Function)
   @ Base ./reducedim.jl:902
 [3] all(f::Function, a::Vector{Vector})
   @ Base ./reducedim.jl:902
 [4] test_approx(z::AbstractZero, x::AbstractArray{<:AbstractArray}, msg::Any; kwargs::Base.Pairs{Symbol, V, Tuple{Vararg{Symbol, N}}, NamedTuple{names, T}} where {V, N, names, T<:Tuple{Vararg{Any, N}}})
   @ ChainRulesTestUtils ~/JuliaEnvs/ChainRulesTestUtils.jl/src/check_result.jl:47
 [5] test_approx (repeats 2 times)
   @ ~/JuliaEnvs/ChainRulesTestUtils.jl/src/check_result.jl:47 [inlined]
 [6] top-level scope
   @ REPL[12]:1

The alternative is to create a new testset, something like:

function test_approx(z::AbstractZero, x::AbstractArray{<:AbstractArray}, msg=""; kwargs...)
    @testset "test_approx($(typeof(z)), $(typeof(x)))" begin
        for el in x
            test_approx(el, z, msg; kwargs...)
        end
    end
end

It's quite verbose, but probably closer to what we want. (Though in principle this could result in very nested testsets)

[devmotion]

The issue is that the test_approx does not return a boolean, but rather does the @test.

Ah sorry, I missed that. Is a separate test set actually needed? It seems there are already some definitions such as https://github.com/alyst/ChainRulesTestUtils.jl/blob/5b9ec32a49a26984112edc6102488538d5efe2ae/src/check_result.jl#L99 that loop over elements without wrapping the individual tests in a test set. But maybe they should?

(As a side remark, it seems the definitions for NoTangent-Tangent are wrong as they return a boolean?)

[mzgubic]

Thanks, correct on both points - I've opened a PR to fix, would be grateful if you could review? #259