Skip to content

Support partial-dimension FFTs - #615

Open
michel2323 wants to merge 1 commit into
mainfrom
ms/partial-fft
Open

Support partial-dimension FFTs#615
michel2323 wants to merge 1 commit into
mainfrom
ms/partial-fft

Conversation

@michel2323

Copy link
Copy Markdown
Member

The oneMKL FFT wrapper previously rejected any transform region that wasn't all dimensions ("Partial dimension FFT not yet supported"). This was purely a wrapper limitation: the oneMKL DFT descriptor can express these via FWD/BWD_STRIDES, NUMBER_OF_TRANSFORMS, and FWD/BWD_DISTANCE, all of which the support library already exposes.

With this PR, plan_fft/plan_bfft (in-place and out-of-place) and rfft/irfft/brfft accept any contiguous region of dimensions, e.g. fft(A, 1), fft(A, 2), or fft(A, (2, 3)) for a 3D array:

  • The four complex plan constructors are unified into _plan_cfft, which programs a region j:k as an R-dimensional transform with explicit column-major strides.
  • Dimensions outside the region are batched: trailing dimensions via NUMBER_OF_TRANSFORMS with the block length as distance; leading dimensions as interleaved transforms with distance 1. When batch dimensions exist on both sides (a "middle" region like dim 2 of a 3D array), the trailing side becomes repeated executions at shifted pointer offsets, since a descriptor has only one distance parameter.
  • Non-contiguous regions (e.g. (1, 3)) throw an informative error instead of the previous blanket rejection.

Two silently-wrong pre-existing paths are also fixed:

  • Multi-dimensional brfft was routed to a non-batched 1D descriptor, so only the first column was transformed and the rest of the output was garbage. It now goes through the complex path.
  • The complex-based inverse real FFT (ComplexBasedRealIFFTPlan) zero-padded the missing half of the spectrum instead of reconstructing it by conjugate symmetry, giving inaccurate results. It now performs a proper Hermitian reconstruction (using negative-step range indexing, since Base.reverse(A; dims) falls back to scalar indexing for oneArray).

Tests cover partial regions for even/odd sizes in 2D/3D (complex forward/inverse, in-place, real forward/inverse) against FFTW, plus N-D irfft/brfft which was previously only tested in 1D. The fft testsuite passes 188/188 on an Arc A750.

fft/bfft plans (in-place and out-of-place) and rfft/irfft/brfft now accept
any contiguous region of dimensions, e.g. fft(A, 1) or fft(A, (2, 3)) for
a 3D array. Regions are expressed through the oneMKL DFT descriptor using
explicit strides plus NUMBER_OF_TRANSFORMS and distances; regions with
batch dimensions on both sides are handled by repeated executions at
shifted pointer offsets, since a descriptor only has a single batch
distance. Non-contiguous regions (e.g. (1, 3)) throw an informative error.

Also fixes two silently-wrong paths: multi-dimensional brfft was routed to
a non-batched 1D descriptor and only transformed the first column, and the
complex-based inverse real FFT zero-padded the spectrum instead of
reconstructing conjugate symmetry.
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.94595% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.90%. Comparing base (38876c5) to head (388319a).

Files with missing lines Patch % Lines
lib/mkl/fft.jl 95.94% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #615      +/-   ##
==========================================
+ Coverage   79.97%   80.90%   +0.92%     
==========================================
  Files          50       50              
  Lines        3496     3488       -8     
==========================================
+ Hits         2796     2822      +26     
+ Misses        700      666      -34     

☔ 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