Skip to content

Fix method ambiguity with FFTW.jl for plan_rfft/plan_brfft - #131

Open
pankgeorg wants to merge 1 commit into
JuliaMath:mainfrom
JuliaComputing:fix/fftw-coexistence
Open

Fix method ambiguity with FFTW.jl for plan_rfft/plan_brfft#131
pankgeorg wants to merge 1 commit into
JuliaMath:mainfrom
JuliaComputing:fix/fftw-coexistence

Conversation

@pankgeorg

Copy link
Copy Markdown

With FFTW.jl and FFTA loaded in the same session, plan_rfft(::Vector{Float64}, ::Int) — and therefore rfft(x) — is a method ambiguity:

MethodError: plan_rfft(::Vector{Float64}, ::Int64) is ambiguous.
  plan_rfft(X::StridedArray{Float64, N}, region; ...) @ FFTW
  plan_rfft(x::AbstractArray{T, N}, region::Union{Int64, NTuple{N, Int64} where N, AbstractVector{Int64}}; ...) @ FFTA

FFTW's method is more specific in the array argument, FFTA's in the region argument, so neither wins. Same for plan_brfft (d::Integer vs len::Int). plan_fft/plan_bfft were fine because their region argument is unannotated.

This leaves region unannotated on the AbstractFFTs.plan_rfft/plan_brfft entry points (as plan_fft already does) and normalises it in an internal _plan_rfft/_plan_brfft, so that FFTW's StridedArray methods are strictly more specific and take over when loaded — which is what AbstractFFTs intends (ref #66 for the discussion of that behaviour). FFTA's methods remain reachable for non-strided arrays.

A regression test loads FFTW.jl and checks rfft/irfft/fft work and that no plan_* ambiguities exist. It runs in a subprocess, because using FFTW in the test process would make every other test in the suite exercise FFTW instead of FFTA. This adds FFTW as a test-only dependency; happy to drop the test if that's unwanted.

Found while writing the benchmark suite in #128, where FFTA and FFTW are loaded together.

With FFTW.jl loaded alongside FFTA, plan_rfft(::Vector{Float64}, ::Int) was
ambiguous between FFTW's StridedArray method and FFTA's method annotated
with region::RegionTypes, turning rfft(x) into a MethodError. Leave region
unannotated on the AbstractFFTs entry points (as plan_fft already does) and
normalise it in an internal function, so FFTW's methods are strictly more
specific and take over as AbstractFFTs intends.

A coexistence test runs in a subprocess (loading FFTW in the test process
would make every other test exercise FFTW).
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.64%. Comparing base (7aeb327) to head (2f58ee0).

Files with missing lines Patch % Lines
src/plan.jl 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #131      +/-   ##
==========================================
- Coverage   98.80%   98.64%   -0.17%     
==========================================
  Files           5        5              
  Lines         585      589       +4     
==========================================
+ Hits          578      581       +3     
- Misses          7        8       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant