From 10e12af9c524fec4947dd9a4a12c5c03f18548dc Mon Sep 17 00:00:00 2001 From: Gaurav Arya Date: Sat, 16 Jul 2022 01:36:04 -0400 Subject: [PATCH 1/6] Immutable ScaledPlan --- src/definitions.jl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/definitions.jl b/src/definitions.jl index fe62e96..ac9a4ba 100644 --- a/src/definitions.jl +++ b/src/definitions.jl @@ -234,6 +234,8 @@ plan_rfft(x::AbstractArray, region; kws...) = plan_rfft(realfloat(x), region; kw _pinv_type(p::Plan) = typeof([plan_inv(x) for x in typeof(p)[]]) pinv_type(p::Plan) = eltype(_pinv_type(p)) +function plan_inv end + inv(p::Plan) = isdefined(p, :pinv) ? p.pinv::pinv_type(p) : (p.pinv = plan_inv(p)) \(p::Plan, x::AbstractArray) = inv(p) * x @@ -243,10 +245,9 @@ LinearAlgebra.ldiv!(y::AbstractArray, p::Plan, x::AbstractArray) = LinearAlgebra # implementations only need to provide the unnormalized backwards FFT, # similar to FFTW, and we do the scaling generically to get the ifft: -mutable struct ScaledPlan{T,P,N} <: Plan{T} +struct ScaledPlan{T,P,N} <: Plan{T} p::P scale::N # not T, to avoid unnecessary promotion to Complex - pinv::Plan ScaledPlan{T,P,N}(p, scale) where {T,P,N} = new(p, scale) end ScaledPlan{T}(p::P, scale::N) where {T,P,N} = ScaledPlan{T,P,N}(p, scale) @@ -278,7 +279,7 @@ plan_ifft(x::AbstractArray, region; kws...) = plan_ifft!(x::AbstractArray, region; kws...) = ScaledPlan(plan_bfft!(x, region; kws...), normalization(x, region)) -plan_inv(p::ScaledPlan) = ScaledPlan(inv(p.p), inv(p.scale)) +inv(p::ScaledPlan) = ScaledPlan(inv(p.p), inv(p.scale)) LinearAlgebra.mul!(y::AbstractArray, p::ScaledPlan, x::AbstractArray) = LinearAlgebra.lmul!(p.scale, LinearAlgebra.mul!(y, p.p, x)) From 4b57f5406377eedbca2199771eb2511b8a51fdac Mon Sep 17 00:00:00 2001 From: gaurav-arya Date: Thu, 18 Aug 2022 13:57:11 -0400 Subject: [PATCH 2/6] Support `plan_inv` for ScaledPlan's (#77) * Add plan_inv for ScaledPlan's * Add tests of plan_inv behaviour * Get type T more correct for real FFT test plans --- src/definitions.jl | 2 ++ test/runtests.jl | 52 ++++++++++++++++++++++++++++++---------------- test/testplans.jl | 8 +++---- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/definitions.jl b/src/definitions.jl index ac9a4ba..4532650 100644 --- a/src/definitions.jl +++ b/src/definitions.jl @@ -279,6 +279,8 @@ plan_ifft(x::AbstractArray, region; kws...) = plan_ifft!(x::AbstractArray, region; kws...) = ScaledPlan(plan_bfft!(x, region; kws...), normalization(x, region)) +plan_inv(p::ScaledPlan) = ScaledPlan(plan_inv(p.p), inv(p.scale)) +# Don't cache inverse of scaled plan (only inverse of inner plan) inv(p::ScaledPlan) = ScaledPlan(inv(p.p), inv(p.scale)) LinearAlgebra.mul!(y::AbstractArray, p::ScaledPlan, x::AbstractArray) = diff --git a/test/runtests.jl b/test/runtests.jl index 623d625..4d402c5 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -56,11 +56,15 @@ end dims = ndims(x) y = AbstractFFTs.fft(x, dims) @test y ≈ fftw_fft - P = plan_fft(x, dims) - @test eltype(P) === ComplexF64 - @test P * x ≈ fftw_fft - @test P \ (P * x) ≈ x - @test fftdims(P) == dims + # test plan_fft and also inv and plan_inv of plan_ifft, which should all give + # functionally identical plans + for P in [plan_fft(x, dims), inv(plan_ifft(x, dims)), + AbstractFFTs.plan_inv(plan_ifft(x, dims))] + @test eltype(P) === ComplexF64 + @test P * x ≈ fftw_fft + @test P \ (P * x) ≈ x + @test fftdims(P) == dims + end fftw_bfft = complex.(size(x, dims) .* x) @test AbstractFFTs.bfft(y, dims) ≈ fftw_bfft @@ -71,10 +75,14 @@ end fftw_ifft = complex.(x) @test AbstractFFTs.ifft(y, dims) ≈ fftw_ifft - P = plan_ifft(x, dims) - @test P * y ≈ fftw_ifft - @test P \ (P * y) ≈ y - @test fftdims(P) == dims + # test plan_ifft and also inv and plan_inv of plan_fft, which should all give + # functionally identical plans + for P in [plan_ifft(x, dims), inv(plan_fft(x, dims)), + AbstractFFTs.plan_inv(plan_fft(x, dims))] + @test P * y ≈ fftw_ifft + @test P \ (P * y) ≈ y + @test fftdims(P) == dims + end # real FFT fftw_rfft = fftw_fft[ @@ -83,11 +91,15 @@ end ] ry = AbstractFFTs.rfft(x, dims) @test ry ≈ fftw_rfft - P = plan_rfft(x, dims) - @test eltype(P) === Int - @test P * x ≈ fftw_rfft - @test P \ (P * x) ≈ x - @test fftdims(P) == dims + # test plan_rfft and also inv and plan_inv of plan_irfft, which should all give + # functionally identical plans + for P in [plan_rfft(x, dims), inv(plan_irfft(ry, size(x, dims), dims)), + AbstractFFTs.plan_inv(plan_irfft(ry, size(x, dims), dims))] + @test eltype(P) <: Real + @test P * x ≈ fftw_rfft + @test P \ (P * x) ≈ x + @test fftdims(P) == dims + end fftw_brfft = complex.(size(x, dims) .* x) @test AbstractFFTs.brfft(ry, size(x, dims), dims) ≈ fftw_brfft @@ -98,10 +110,14 @@ end fftw_irfft = complex.(x) @test AbstractFFTs.irfft(ry, size(x, dims), dims) ≈ fftw_irfft - P = plan_irfft(ry, size(x, dims), dims) - @test P * ry ≈ fftw_irfft - @test P \ (P * ry) ≈ ry - @test fftdims(P) == dims + # test plan_rfft and also inv and plan_inv of plan_irfft, which should all give + # functionally identical plans + for P in [plan_irfft(ry, size(x, dims), dims), inv(plan_rfft(x, dims)), + AbstractFFTs.plan_inv(plan_rfft(x, dims))] + @test P * ry ≈ fftw_irfft + @test P \ (P * ry) ≈ ry + @test fftdims(P) == dims + end end end diff --git a/test/testplans.jl b/test/testplans.jl index 7abecfe..31609a9 100644 --- a/test/testplans.jl +++ b/test/testplans.jl @@ -92,11 +92,11 @@ Base.:*(p::InverseTestPlan, x::AbstractArray) = mul!(similar(x, complex(float(el mutable struct TestRPlan{T,N} <: Plan{T} region sz::NTuple{N,Int} - pinv::Plan{T} + pinv::Plan{Complex{T}} TestRPlan{T}(region, sz::NTuple{N,Int}) where {T,N} = new{T,N}(region, sz) end -mutable struct InverseTestRPlan{T,N} <: Plan{T} +mutable struct InverseTestRPlan{T,N} <: Plan{Complex{T}} d::Int region sz::NTuple{N,Int} @@ -107,10 +107,10 @@ mutable struct InverseTestRPlan{T,N} <: Plan{T} end end -function AbstractFFTs.plan_rfft(x::AbstractArray{T}, region; kwargs...) where {T} +function AbstractFFTs.plan_rfft(x::AbstractArray{T}, region; kwargs...) where {T<:Real} return TestRPlan{T}(region, size(x)) end -function AbstractFFTs.plan_brfft(x::AbstractArray{T}, d, region; kwargs...) where {T} +function AbstractFFTs.plan_brfft(x::AbstractArray{Complex{T}}, d, region; kwargs...) where {T} return InverseTestRPlan{T}(d, region, size(x)) end function AbstractFFTs.plan_inv(p::TestRPlan{T,N}) where {T,N} From 0918c3c36424865854bc498a5f6b78a9bb09bece Mon Sep 17 00:00:00 2001 From: David Widmann Date: Thu, 18 Aug 2022 21:14:44 +0200 Subject: [PATCH 3/6] Add downstream integration tests (#76) --- .github/workflows/IntegrationTest.yml | 49 +++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 .github/workflows/IntegrationTest.yml diff --git a/.github/workflows/IntegrationTest.yml b/.github/workflows/IntegrationTest.yml new file mode 100644 index 0000000..34d8ca4 --- /dev/null +++ b/.github/workflows/IntegrationTest.yml @@ -0,0 +1,49 @@ +name: IntegrationTest + +on: + pull_request: + push: + branches: + - master + tags: '*' + +jobs: + test: + name: ${{ matrix.package.repo }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + julia-version: [1] + os: [ubuntu-latest] + package: + - {user: JuliaMath, repo: FFTW.jl} + steps: + - uses: actions/checkout@v2 + - uses: julia-actions/setup-julia@v1 + with: + version: ${{ matrix.julia-version }} + arch: x64 + - uses: julia-actions/julia-buildpkg@latest + - name: Clone Downstream + uses: actions/checkout@v2 + with: + repository: ${{ matrix.package.user }}/${{ matrix.package.repo }} + path: downstream + - name: Load this and run the downstream tests + shell: julia --project=downstream {0} + run: | + using Pkg + try + # force it to use this PR's version of the package + Pkg.develop(PackageSpec(path=".")) # resolver may fail with main deps + Pkg.update() + Pkg.test() # resolver may fail with test time deps + catch err + err isa Pkg.Resolve.ResolverError || rethrow() + # If we can't resolve that means this is incompatible by SemVer and this is fine + # It means we marked this as a breaking change, so we don't need to worry about + # Mistakenly introducing a breaking change, as we have intentionally made one + @info "Not compatible with this release. No problem." exception=err + exit(0) # Exit immediately, as a success + end From 20458e42a26640e17a8d0414f296ec26f754b268 Mon Sep 17 00:00:00 2001 From: Hendrik Ranocha Date: Fri, 4 Nov 2022 17:13:24 +0100 Subject: [PATCH 4/6] Create Invalidations.yml (#79) This is based on https://github.com/julia-actions/julia-invalidations. Adding such checks came up in https://discourse.julialang.org/t/potential-performance-regressions-in-julia-1-8-for-special-un-precompiled-type-dispatches-and-how-to-fix-them/86359. I suggest to add this check here since this package is widely used as a dependency. --- .github/workflows/Invalidations.yml | 40 +++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 .github/workflows/Invalidations.yml diff --git a/.github/workflows/Invalidations.yml b/.github/workflows/Invalidations.yml new file mode 100644 index 0000000..770a9b1 --- /dev/null +++ b/.github/workflows/Invalidations.yml @@ -0,0 +1,40 @@ +name: Invalidations + +on: + pull_request: + +concurrency: + # Skip intermediate builds: always. + # Cancel intermediate builds: always. + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + evaluate: + # Only run on PRs to the default branch. + # In the PR trigger above branches can be specified only explicitly whereas this check should work for master, main, or any other default branch + if: github.base_ref == github.event.repository.default_branch + runs-on: ubuntu-latest + steps: + - uses: julia-actions/setup-julia@v1 + with: + version: '1' + - uses: actions/checkout@v3 + - uses: julia-actions/julia-buildpkg@v1 + - uses: julia-actions/julia-invalidations@v1 + id: invs_pr + + - uses: actions/checkout@v3 + with: + ref: ${{ github.event.repository.default_branch }} + - uses: julia-actions/julia-buildpkg@v1 + - uses: julia-actions/julia-invalidations@v1 + id: invs_default + + - name: Report invalidation counts + run: | + echo "Invalidations on default branch: ${{ steps.invs_default.outputs.total }} (${{ steps.invs_default.outputs.deps }} via deps)" >> $GITHUB_STEP_SUMMARY + echo "This branch: ${{ steps.invs_pr.outputs.total }} (${{ steps.invs_pr.outputs.deps }} via deps)" >> $GITHUB_STEP_SUMMARY + - name: Check whether the number of invalidations increased + if: steps.invs_pr.outputs.total > steps.invs_default.outputs.total + run: exit 1 From 7d698db082f4ea1898afe40fbf2755e915ee5ff7 Mon Sep 17 00:00:00 2001 From: Alex Arslan Date: Fri, 4 Nov 2022 09:26:26 -0700 Subject: [PATCH 5/6] Update badges and use LaTeX for math - We're using GHA + Codecov rather than Travis + Coveralls - GitHub now renders LaTeX math in Markdown --- README.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 5b33c59..4a884c1 100644 --- a/README.md +++ b/README.md @@ -2,12 +2,12 @@ A general framework for fast Fourier transforms (FFTs) in Julia. -[![Travis](https://travis-ci.org/JuliaMath/AbstractFFTs.jl.svg?branch=master)](https://travis-ci.org/JuliaMath/AbstractFFTs.jl) -[![Coveralls](https://coveralls.io/repos/github/JuliaMath/AbstractFFTs.jl/badge.svg?branch=master)](https://coveralls.io/github/JuliaMath/AbstractFFTs.jl?branch=master) +[![GHA](https://github.com/JuliaMath/AbstractFFTs.jl/workflows/CI/badge.svg)](https://github.com/JuliaMath/AbstractFFTs.jl/actions?query=workflow%3ACI+branch%3Amaster) +[![Codecov](http://codecov.io/github/JuliaMath/AbstractFFTs.jl/coverage.svg?branch=master)](http://codecov.io/github/JuliaMath/AbstractFFTs.jl?branch=master) Documentation: [![](https://img.shields.io/badge/docs-stable-blue.svg)](https://JuliaMath.github.io/AbstractFFTs.jl/stable) -[![](https://img.shields.io/badge/docs-latest-blue.svg)](https://JuliaMath.github.io/AbstractFFTs.jl/latest) +[![](https://img.shields.io/badge/docs-latest-blue.svg)](https://JuliaMath.github.io/AbstractFFTs.jl/dev) This package is mainly not intended to be used directly. Instead, developers of packages that implement FFTs (such as [FFTW.jl](https://github.com/JuliaMath/FFTW.jl) or [FastTransforms.jl](https://github.com/JuliaApproximation/FastTransforms.jl)) @@ -36,5 +36,6 @@ To define a new FFT implementation in your own module, you should * You can also define similar methods of `plan_rfft` and `plan_brfft` for real-input FFTs. -The normalization convention for your FFT should be that it computes yₖ = ∑ⱼ xⱼ exp(-2πi jk/n) for a transform of -length n, and the "backwards" (unnormalized inverse) transform computes the same thing but with exp(+2πi jk/n). +The normalization convention for your FFT should be that it computes $y_k = \sum_j \exp\(-2 \pi i \cdot \frac{j k}{n}\)$ +for a transform of length $n$, and the "backwards" (unnormalized inverse) transform computes the same thing but with +$\exp\(+2 \pi i \cdot \frac{j k}{n}\)$. From b2dd69cc19b4dfaeed377fd9f81d7631459b079d Mon Sep 17 00:00:00 2001 From: David Widmann Date: Tue, 7 Mar 2023 22:36:43 +0100 Subject: [PATCH 6/6] Make ChainRulesCore a weak dependency on Julia >= 1.9 (#85) * Make ChainRulesCore a weak dependency on Julia >= 1.9 * Qualify `normalization` * Check on nightly if extension works correctly --- .github/workflows/CI.yml | 2 +- Project.toml | 11 +++++++++-- .../AbstractFFTsChainRulesCoreExt.jl | 12 +++++++++--- src/AbstractFFTs.jl | 7 ++++--- 4 files changed, 23 insertions(+), 9 deletions(-) rename src/chainrules.jl => ext/AbstractFFTsChainRulesCoreExt.jl (96%) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 8d43117..1444ca5 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -15,7 +15,7 @@ jobs: version: - '1.0' - '1' -# - 'nightly' + - 'nightly' os: - ubuntu-latest - macOS-latest diff --git a/Project.toml b/Project.toml index a639c5d..572c7a2 100644 --- a/Project.toml +++ b/Project.toml @@ -1,20 +1,27 @@ name = "AbstractFFTs" uuid = "621f4979-c628-5d54-868e-fcf4e3e8185c" -version = "1.2.1" +version = "1.3.0" [deps] ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4" LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" +[weakdeps] +ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4" + +[extensions] +AbstractFFTsChainRulesCoreExt = "ChainRulesCore" + [compat] ChainRulesCore = "1" julia = "^1.0" [extras] +ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4" ChainRulesTestUtils = "cdddcdb0-9152-4a09-a978-84456f9df70a" Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" Unitful = "1986cc42-f94f-5a68-af5c-568840ba703d" [targets] -test = ["ChainRulesTestUtils", "Random", "Test", "Unitful"] +test = ["ChainRulesCore", "ChainRulesTestUtils", "Random", "Test", "Unitful"] diff --git a/src/chainrules.jl b/ext/AbstractFFTsChainRulesCoreExt.jl similarity index 96% rename from src/chainrules.jl rename to ext/AbstractFFTsChainRulesCoreExt.jl index 97d4d22..f0c788e 100644 --- a/src/chainrules.jl +++ b/ext/AbstractFFTsChainRulesCoreExt.jl @@ -1,4 +1,8 @@ -# ffts +module AbstractFFTsChainRulesCoreExt + +using AbstractFFTs +import ChainRulesCore + function ChainRulesCore.frule((_, Δx, _), ::typeof(fft), x::AbstractArray, dims) y = fft(x, dims) Δy = fft(Δx, dims) @@ -46,7 +50,7 @@ function ChainRulesCore.frule((_, Δx, _), ::typeof(ifft), x::AbstractArray, dim end function ChainRulesCore.rrule(::typeof(ifft), x::AbstractArray, dims) y = ifft(x, dims) - invN = normalization(y, dims) + invN = AbstractFFTs.normalization(y, dims) project_x = ChainRulesCore.ProjectTo(x) function ifft_pullback(ȳ) x̄ = project_x(invN .* fft(ChainRulesCore.unthunk(ȳ), dims)) @@ -66,7 +70,7 @@ function ChainRulesCore.rrule(::typeof(irfft), x::AbstractArray, d::Int, dims) # compute scaling factors halfdim = first(dims) n = size(x, halfdim) - invN = normalization(y, dims) + invN = AbstractFFTs.normalization(y, dims) twoinvN = 2 * invN scale = reshape( [i == 1 || (i == n && 2 * (i - 1) == d) ? invN : twoinvN for i in 1:n], @@ -150,3 +154,5 @@ function ChainRulesCore.rrule(::typeof(ifftshift), x::AbstractArray, dims) end return y, ifftshift_pullback end + +end # module diff --git a/src/AbstractFFTs.jl b/src/AbstractFFTs.jl index 56d7123..00f6dc2 100644 --- a/src/AbstractFFTs.jl +++ b/src/AbstractFFTs.jl @@ -1,13 +1,14 @@ module AbstractFFTs -import ChainRulesCore - export fft, ifft, bfft, fft!, ifft!, bfft!, plan_fft, plan_ifft, plan_bfft, plan_fft!, plan_ifft!, plan_bfft!, rfft, irfft, brfft, plan_rfft, plan_irfft, plan_brfft, fftdims, fftshift, ifftshift, fftshift!, ifftshift!, Frequencies, fftfreq, rfftfreq include("definitions.jl") -include("chainrules.jl") + +if !isdefined(Base, :get_extension) + include("../ext/AbstractFFTsChainRulesCoreExt.jl") +end end # module