From 00736a8c51496f4c484f1bc752c10bccbec15216 Mon Sep 17 00:00:00 2001 From: Gaurav Arya Date: Sun, 3 Sep 2023 22:06:52 -0400 Subject: [PATCH 1/5] Fixes to test suite to support CUDA arrays --- Project.toml | 1 + ext/AbstractFFTsTestExt.jl | 69 +++++++++++++++++++++----------------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/Project.toml b/Project.toml index e8fb51f..7de5d36 100644 --- a/Project.toml +++ b/Project.toml @@ -5,6 +5,7 @@ version = "1.5.0" [deps] ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4" LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" +Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" [weakdeps] diff --git a/ext/AbstractFFTsTestExt.jl b/ext/AbstractFFTsTestExt.jl index aad9d7d..db00e80 100644 --- a/ext/AbstractFFTsTestExt.jl +++ b/ext/AbstractFFTsTestExt.jl @@ -6,6 +6,7 @@ using AbstractFFTs using AbstractFFTs: TestUtils using AbstractFFTs.LinearAlgebra using Test +import Random # Ground truth x_fft computed using FFTW library const TEST_CASES = ( @@ -52,7 +53,8 @@ const TEST_CASES = ( ) -function TestUtils.test_plan(P::AbstractFFTs.Plan, x::AbstractArray, x_transformed::AbstractArray; inplace_plan=false, copy_input=false) +function TestUtils.test_plan(P::AbstractFFTs.Plan, x::AbstractArray, x_transformed::AbstractArray; + inplace_plan=false, copy_input=false, test_wrappers=true) _copy = copy_input ? copy : identity @test size(P) == size(x) if !inplace_plan @@ -61,7 +63,9 @@ function TestUtils.test_plan(P::AbstractFFTs.Plan, x::AbstractArray, x_transform _x_out = similar(P * _copy(x)) @test mul!(_x_out, P, _copy(x)) ≈ x_transformed @test _x_out ≈ x_transformed - @test P * view(_copy(x), axes(x)...) ≈ x_transformed # test view input + if test_wrappers + @test P * view(_copy(x), axes(x)...) ≈ x_transformed # test view input + end else _x = copy(x) @test P * _copy(_x) ≈ x_transformed @@ -71,9 +75,10 @@ function TestUtils.test_plan(P::AbstractFFTs.Plan, x::AbstractArray, x_transform end end -function TestUtils.test_plan_adjoint(P::AbstractFFTs.Plan, x::AbstractArray; real_plan=false, copy_input=false) +function TestUtils.test_plan_adjoint(P::AbstractFFTs.Plan, x::AbstractArray; + real_plan=false, copy_input=false, test_wrappers=true) _copy = copy_input ? copy : identity - y = rand(eltype(P * _copy(x)), size(P * _copy(x))) + y = Random.rand!(P * _copy(x)) # test basic properties @test eltype(P') === eltype(y) @test (P')' === P # test adjoint of adjoint @@ -87,11 +92,13 @@ function TestUtils.test_plan_adjoint(P::AbstractFFTs.Plan, x::AbstractArray; rea @test _component_dot(y, P * _copy(x)) ≈ _component_dot(P' * _copy(y), x) @test _component_dot(x, P \ _copy(y)) ≈ _component_dot(P' \ _copy(x), y) end - @test P' * view(_copy(y), axes(y)...) ≈ P' * _copy(y) # test view input (AbstractFFTs.jl#112) + if test_wrappers + @test P' * view(_copy(y), axes(y)...) ≈ P' * _copy(y) # test view input (AbstractFFTs.jl#112) + end @test_throws MethodError mul!(x, P', y) end -function TestUtils.test_complex_ffts(ArrayType=Array; test_inplace=true, test_adjoint=true) +function TestUtils.test_complex_ffts(ArrayType=Array; test_inplace=true, test_adjoint=true, test_wrappers=true) @testset "correctness of fft, bfft, ifft" begin for test_case in TEST_CASES _x, dims, _x_fft = copy(test_case.x), test_case.dims, copy(test_case.x_fft) @@ -111,18 +118,18 @@ function TestUtils.test_complex_ffts(ArrayType=Array; test_inplace=true, test_ad for P in (plan_fft(similar(x_complexf), dims), (_inv(plan_ifft(similar(x_complexf), dims)) for _inv in (inv, AbstractFFTs.plan_inv))...) @test eltype(P) <: Complex - @test fftdims(P) == dims - TestUtils.test_plan(P, x_complexf, x_fft) + @test collect(fftdims(P))[:] == collect(dims)[:] # compare as iterables + TestUtils.test_plan(P, x_complexf, x_fft; test_wrappers=test_wrappers) if test_adjoint @test fftdims(P') == fftdims(P) - TestUtils.test_plan_adjoint(P, x_complexf) + TestUtils.test_plan_adjoint(P, x_complexf, test_wrappers=test_wrappers) end end if test_inplace # test IIP plans for P in (plan_fft!(similar(x_complexf), dims), (_inv(plan_ifft!(similar(x_complexf), dims)) for _inv in (inv, AbstractFFTs.plan_inv))...) - TestUtils.test_plan(P, x_complexf, x_fft; inplace_plan=true) + TestUtils.test_plan(P, x_complexf, x_fft; inplace_plan=true, test_wrappers=test_wrappers) end end @@ -137,17 +144,17 @@ function TestUtils.test_complex_ffts(ArrayType=Array; test_inplace=true, test_ad # test OOP plans. Just 1 plan to test, but we use a for loop for consistent style for P in (plan_bfft(similar(x_fft), dims),) @test eltype(P) <: Complex - @test fftdims(P) == dims - TestUtils.test_plan(P, x_fft, x_scaled) + @test collect(fftdims(P))[:] == collect(dims)[:] # compare as iterables + TestUtils.test_plan(P, x_fft, x_scaled; test_wrappers=test_wrappers) if test_adjoint - TestUtils.test_plan_adjoint(P, x_fft) + TestUtils.test_plan_adjoint(P, x_fft, test_wrappers=test_wrappers) end end # test IIP plans for P in (plan_bfft!(similar(x_fft), dims),) @test eltype(P) <: Complex - @test fftdims(P) == dims - TestUtils.test_plan(P, x_fft, x_scaled; inplace_plan=true) + @test collect(fftdims(P))[:] == collect(dims)[:] # compare as iterables + TestUtils.test_plan(P, x_fft, x_scaled; inplace_plan=true, test_wrappers=test_wrappers) end # IFFT @@ -161,10 +168,10 @@ function TestUtils.test_complex_ffts(ArrayType=Array; test_inplace=true, test_ad for P in (plan_ifft(similar(x_complexf), dims), (_inv(plan_fft(similar(x_complexf), dims)) for _inv in (inv, AbstractFFTs.plan_inv))...) @test eltype(P) <: Complex - @test fftdims(P) == dims - TestUtils.test_plan(P, x_fft, x) + @test collect(fftdims(P))[:] == collect(dims)[:] # compare as iterables + TestUtils.test_plan(P, x_fft, x; test_wrappers=test_wrappers) if test_adjoint - TestUtils.test_plan_adjoint(P, x_fft) + TestUtils.test_plan_adjoint(P, x_fft; test_wrappers=test_wrappers) end end # test IIP plans @@ -172,22 +179,22 @@ function TestUtils.test_complex_ffts(ArrayType=Array; test_inplace=true, test_ad for P in (plan_ifft!(similar(x_complexf), dims), (_inv(plan_fft!(similar(x_complexf), dims)) for _inv in (inv, AbstractFFTs.plan_inv))...) @test eltype(P) <: Complex - @test fftdims(P) == dims - TestUtils.test_plan(P, x_fft, x; inplace_plan=true) + @test collect(fftdims(P))[:] == collect(dims)[:] # compare as iterables + TestUtils.test_plan(P, x_fft, x; inplace_plan=true, test_wrappers=test_wrappers) end end end end end -function TestUtils.test_real_ffts(ArrayType=Array; test_adjoint=true, copy_input=false) +function TestUtils.test_real_ffts(ArrayType=Array; test_adjoint=true, copy_input=false, test_wrappers=true) @testset "correctness of rfft, brfft, irfft" begin for test_case in TEST_CASES _x, dims, _x_fft = copy(test_case.x), test_case.dims, copy(test_case.x_fft) x = convert(ArrayType, _x) # dummy array that will be passed to plans x_real = float.(x) # for testing mutating real FFTs x_fft = convert(ArrayType, _x_fft) - x_rfft = collect(selectdim(x_fft, first(dims), 1:(size(x_fft, first(dims)) ÷ 2 + 1))) + x_rfft = convert(ArrayType, collect(selectdim(x_fft, first(dims), 1:(size(x_fft, first(dims)) ÷ 2 + 1)))) if !(eltype(x) <: Real) continue @@ -198,10 +205,10 @@ function TestUtils.test_real_ffts(ArrayType=Array; test_adjoint=true, copy_input for P in (plan_rfft(similar(x_real), dims), (_inv(plan_irfft(similar(x_rfft), size(x, first(dims)), dims)) for _inv in (inv, AbstractFFTs.plan_inv))...) @test eltype(P) <: Real - @test fftdims(P) == dims - TestUtils.test_plan(P, x_real, x_rfft; copy_input=copy_input) + @test collect(fftdims(P))[:] == collect(dims)[:] # compare as iterables + TestUtils.test_plan(P, x_real, x_rfft; copy_input=copy_input, test_wrappers=test_wrappers) if test_adjoint - TestUtils.test_plan_adjoint(P, x_real; real_plan=true, copy_input=copy_input) + TestUtils.test_plan_adjoint(P, x_real; real_plan=true, copy_input=copy_input, test_wrappers=test_wrappers) end end @@ -210,10 +217,10 @@ function TestUtils.test_real_ffts(ArrayType=Array; test_adjoint=true, copy_input @test brfft(x_rfft, size(x, first(dims)), dims) ≈ x_scaled for P in (plan_brfft(similar(x_rfft), size(x, first(dims)), dims),) @test eltype(P) <: Complex - @test fftdims(P) == dims - TestUtils.test_plan(P, x_rfft, x_scaled; copy_input=copy_input) + @test collect(fftdims(P))[:] == collect(dims)[:] # compare as iterables + TestUtils.test_plan(P, x_rfft, x_scaled; copy_input=copy_input, test_wrappers=test_wrappers) if test_adjoint - TestUtils.test_plan_adjoint(P, x_rfft; real_plan=true, copy_input=copy_input) + TestUtils.test_plan_adjoint(P, x_rfft; real_plan=true, copy_input=copy_input, test_wrappers=test_wrappers) end end @@ -222,10 +229,10 @@ function TestUtils.test_real_ffts(ArrayType=Array; test_adjoint=true, copy_input for P in (plan_irfft(similar(x_rfft), size(x, first(dims)), dims), (_inv(plan_rfft(similar(x_real), dims)) for _inv in (inv, AbstractFFTs.plan_inv))...) @test eltype(P) <: Complex - @test fftdims(P) == dims - TestUtils.test_plan(P, x_rfft, x; copy_input=copy_input) + @test collect(fftdims(P))[:] == collect(dims)[:] # compare as iterables + TestUtils.test_plan(P, x_rfft, x; copy_input=copy_input, test_wrappers=test_wrappers) if test_adjoint - TestUtils.test_plan_adjoint(P, x_rfft; real_plan=true, copy_input=copy_input) + TestUtils.test_plan_adjoint(P, x_rfft; real_plan=true, copy_input=copy_input, test_wrappers=test_wrappers) end end end From e6f6d91c18b21af2142f961225b139f6ca541484 Mon Sep 17 00:00:00 2001 From: Gaurav Arya Date: Sun, 3 Sep 2023 22:17:32 -0400 Subject: [PATCH 2/5] Document test_wrappers --- src/TestUtils.jl | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/TestUtils.jl b/src/TestUtils.jl index 8816e1f..465e6ce 100644 --- a/src/TestUtils.jl +++ b/src/TestUtils.jl @@ -3,7 +3,7 @@ module TestUtils import ..AbstractFFTs """ - TestUtils.test_complex_ffts(ArrayType=Array; test_inplace=true, test_adjoint=true) + TestUtils.test_complex_ffts(ArrayType=Array; test_inplace=true, test_adjoint=true, test_wrappers=true) Run tests to verify correctness of FFT, BFFT, and IFFT functionality using a particular backend plan implementation. The backend implementation is assumed to be loaded prior to calling this function. @@ -15,11 +15,12 @@ The backend implementation is assumed to be loaded prior to calling this functio `convert(ArrayType, ...)`. - `test_inplace=true`: whether to test in-place plans. - `test_adjoint=true`: whether to test [plan adjoints](api.md#Base.adjoint). +- `test_wrappers=true`: whether to test any wrapper array inputs such as views. """ function test_complex_ffts end """ - TestUtils.test_real_ffts(ArrayType=Array; test_adjoint=true, copy_input=false) + TestUtils.test_real_ffts(ArrayType=Array; test_adjoint=true, copy_input=false, test_wrappers=true) Run tests to verify correctness of RFFT, BRFFT, and IRFFT functionality using a particular backend plan implementation. The backend implementation is assumed to be loaded prior to calling this function. @@ -32,18 +33,21 @@ The backend implementation is assumed to be loaded prior to calling this functio - `test_adjoint=true`: whether to test [plan adjoints](api.md#Base.adjoint). - `copy_input=false`: whether to copy the input before applying the plan in tests, to accomodate for [input-mutating behaviour of real FFTW plans](https://github.com/JuliaMath/AbstractFFTs.jl/issues/101). +- `test_wrappers=true`: whether to test any wrapper array inputs such as views. """ function test_real_ffts end # Always copy input before application due to FFTW real plans possibly mutating input (AbstractFFTs.jl#101) """ TestUtils.test_plan(P::Plan, x::AbstractArray, x_transformed::AbstractArray; - inplace_plan=false, copy_input=false) + inplace_plan=false, copy_input=false, test_wrappers=true) Test basic properties of a plan `P` given an input array `x` and expected output `x_transformed`. Because [real FFTW plans may mutate their input in some cases](https://github.com/JuliaMath/AbstractFFTs.jl/issues/101), we allow specifying `copy_input=true` to allow for this behaviour in tests by copying the input before applying the plan. +We also allow specifying `test_wrappers=false` to skip testing wrapper array inputs such as views, which may cause ambiguity +issues for some array types currently. """ function test_plan end @@ -57,6 +61,8 @@ Real-to-complex and complex-to-real plans require a slightly modified dot test, The plan is assumed out-of-place, as adjoints are not yet supported for in-place plans. Because [real FFTW plans may mutate their input in some cases](https://github.com/JuliaMath/AbstractFFTs.jl/issues/101), we allow specifying `copy_input=true` to allow for this behaviour in tests by copying the input before applying the plan. +We also allow specifying `test_wrappers=false` to skip testing wrapper array inputs such as views, which may cause ambiguity +issues for some array types currently. """ function test_plan_adjoint end From 31755f0457c371e779096d4bdcdf2cf958f8d41a Mon Sep 17 00:00:00 2001 From: Gaurav Arya Date: Thu, 7 Sep 2023 11:07:58 -0400 Subject: [PATCH 3/5] Apply suggestions from code review --- Project.toml | 1 - ext/AbstractFFTsTestExt.jl | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Project.toml b/Project.toml index 7de5d36..e8fb51f 100644 --- a/Project.toml +++ b/Project.toml @@ -5,7 +5,6 @@ version = "1.5.0" [deps] ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4" LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" -Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" [weakdeps] diff --git a/ext/AbstractFFTsTestExt.jl b/ext/AbstractFFTsTestExt.jl index db00e80..d66994b 100644 --- a/ext/AbstractFFTsTestExt.jl +++ b/ext/AbstractFFTsTestExt.jl @@ -6,7 +6,6 @@ using AbstractFFTs using AbstractFFTs: TestUtils using AbstractFFTs.LinearAlgebra using Test -import Random # Ground truth x_fft computed using FFTW library const TEST_CASES = ( @@ -78,7 +77,7 @@ end function TestUtils.test_plan_adjoint(P::AbstractFFTs.Plan, x::AbstractArray; real_plan=false, copy_input=false, test_wrappers=true) _copy = copy_input ? copy : identity - y = Random.rand!(P * _copy(x)) + y = map(a -> rand(typeof(a)), P * _copy(x)) # generically construct rand array # test basic properties @test eltype(P') === eltype(y) @test (P')' === P # test adjoint of adjoint From e46b8ebbad6e3393c8b2a0f5ef32812281f6c8bc Mon Sep 17 00:00:00 2001 From: Gaurav Arya Date: Sat, 21 Oct 2023 16:29:42 -0400 Subject: [PATCH 4/5] Use skip=!test_wrappers rather than branching on test_wrappers for wrapper tests --- ext/AbstractFFTsTestExt.jl | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ext/AbstractFFTsTestExt.jl b/ext/AbstractFFTsTestExt.jl index d66994b..6c8733d 100644 --- a/ext/AbstractFFTsTestExt.jl +++ b/ext/AbstractFFTsTestExt.jl @@ -62,9 +62,7 @@ function TestUtils.test_plan(P::AbstractFFTs.Plan, x::AbstractArray, x_transform _x_out = similar(P * _copy(x)) @test mul!(_x_out, P, _copy(x)) ≈ x_transformed @test _x_out ≈ x_transformed - if test_wrappers - @test P * view(_copy(x), axes(x)...) ≈ x_transformed # test view input - end + @test P * view(_copy(x), axes(x)...) ≈ x_transformed skip=!test_wrappers # test view input else _x = copy(x) @test P * _copy(_x) ≈ x_transformed @@ -91,9 +89,7 @@ function TestUtils.test_plan_adjoint(P::AbstractFFTs.Plan, x::AbstractArray; @test _component_dot(y, P * _copy(x)) ≈ _component_dot(P' * _copy(y), x) @test _component_dot(x, P \ _copy(y)) ≈ _component_dot(P' \ _copy(x), y) end - if test_wrappers - @test P' * view(_copy(y), axes(y)...) ≈ P' * _copy(y) # test view input (AbstractFFTs.jl#112) - end + @test P' * view(_copy(y), axes(y)...) ≈ P' * _copy(y) skip=!test_wrappers # test view input (AbstractFFTs.jl#112) @test_throws MethodError mul!(x, P', y) end From 2db1545ef0123b2ac164acb7ef800f1fc8676432 Mon Sep 17 00:00:00 2001 From: Gaurav Arya Date: Sat, 21 Oct 2023 16:37:12 -0400 Subject: [PATCH 5/5] Revert "Use skip=!test_wrappers rather than branching on test_wrappers for wrapper tests" This reverts commit e46b8ebbad6e3393c8b2a0f5ef32812281f6c8bc. --- ext/AbstractFFTsTestExt.jl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ext/AbstractFFTsTestExt.jl b/ext/AbstractFFTsTestExt.jl index 6c8733d..d66994b 100644 --- a/ext/AbstractFFTsTestExt.jl +++ b/ext/AbstractFFTsTestExt.jl @@ -62,7 +62,9 @@ function TestUtils.test_plan(P::AbstractFFTs.Plan, x::AbstractArray, x_transform _x_out = similar(P * _copy(x)) @test mul!(_x_out, P, _copy(x)) ≈ x_transformed @test _x_out ≈ x_transformed - @test P * view(_copy(x), axes(x)...) ≈ x_transformed skip=!test_wrappers # test view input + if test_wrappers + @test P * view(_copy(x), axes(x)...) ≈ x_transformed # test view input + end else _x = copy(x) @test P * _copy(_x) ≈ x_transformed @@ -89,7 +91,9 @@ function TestUtils.test_plan_adjoint(P::AbstractFFTs.Plan, x::AbstractArray; @test _component_dot(y, P * _copy(x)) ≈ _component_dot(P' * _copy(y), x) @test _component_dot(x, P \ _copy(y)) ≈ _component_dot(P' \ _copy(x), y) end - @test P' * view(_copy(y), axes(y)...) ≈ P' * _copy(y) skip=!test_wrappers # test view input (AbstractFFTs.jl#112) + if test_wrappers + @test P' * view(_copy(y), axes(y)...) ≈ P' * _copy(y) # test view input (AbstractFFTs.jl#112) + end @test_throws MethodError mul!(x, P', y) end