Skip to content

fix: respect out layout in real-input ifft and in rfft of cast input - #389

Open
antonwolfy wants to merge 3 commits into
masterfrom
fix/out-strided-legacy-bugs
Open

antonwolfy wants to merge 3 commits into
masterfrom
fix/out-strided-legacy-bugs

Conversation

@antonwolfy

@antonwolfy antonwolfy commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes two bugs with a user-provided out that are already on master, plus a related slowdown in rfft. Both were found while reviewing #387.

1. ifft / ifftn of real input with a non-contiguous out

The inverse transform of real input is computed as a forward transform followed by an in-place complex conjugation, using v?Conj(xout_size, xout_data, ...). That call treats out as one contiguous block. Cython passes out to the backend directly whenever its element strides match those of the input, so for a strided or reversed out:

  • part of out is left unconjugated, which gives wrong values;
  • memory outside out is conjugated, which corrupts other data and can segfault.
x = np.arange(32.0)[::2]; base = np.full(32, -7-7j); out = base[::2]
mkl_fft.ifft(x, out=out)        # max error 10.05; 8 elements of `base` outside `out` modified

mkl_fft.ifft(np.arange(16.0)[::-1], out=np.full(16, -7-7j)[::-1])
# wrong values, then a crash (heap corruption)

This affects the 1-D (ifft) and N-D (ifftn over all axes) backends, and is reachable through mkl_fft.interfaces.numpy_fft.

Fix (mklfft.c.src): a new {cfloat,cdouble}_conj_inplace helper keeps the v?Conj fast path for contiguous out. Otherwise it conjugates element by element along the strides of out, the same way the existing "copy conjugate even harmonics" loop walks out.

2. rfft / rfftn of cast input with Fortran-ordered out

_r2c_fft1d_impl decides whether MKL can write straight into out by checking that the input and out are both C- or both F-contiguous. It checks the original x, not x_arr. For dtypes other than float32/float64 (e.g. integer, float16, longdouble), x_arr is a C-contiguous float64 copy. So with Fortran-ordered input and out, MKL was handed a C-ordered input and an F-ordered output:

x = np.asfortranarray(np.arange(336).reshape(6, 7, 8) % 7)
out = np.full((4, 7, 8), -1-1j, order="F")
mkl_fft.rfft(x, axis=0, out=out)   # max error 36; float64 input is fine

Fix (_pydfti.pyx): check the contiguity of x_arr. _c2r_fft1d_impl has the same check, but its cast keeps the input's memory order, so it is left unchanged.

3. Slow rfft of cast Fortran-ordered input

Also on master: the cast in _r2c_fft1d_impl requested NPY_ARRAY_ENSURECOPY, which makes PyArray_FROM_OTF return a C-contiguous copy. For Fortran-ordered input this is a slow reordering copy. With fix 2 alone, out= would also need a C-ordered temporary, copied into the Fortran-ordered out. The cast always copies anyway, since the dtype differs. So this PR drops the flag: the copy keeps the input's memory order, as the cast in _c2r_fft1d_impl already does.

rfft of Fortran-ordered int64 input of shape 128³ along axis 0, on 1 thread:

master fix 2 alone this PR
rfft(x, axis=0, out=out_F) 46 ms (wrong result) 55 ms, 32 MiB peak 5.2 ms, 16 MiB peak
rfft(x, axis=0) 43 ms 39 ms 5.5 ms

The result layout now also matches NumPy, which allocates the output with empty_like and so keeps the input's order. Over 18 combinations of dtype, input order and axis, master matched NumPy in 12; this PR matches in all 18. On master, Fortran-ordered int64/float16 input gave C-ordered results.

Performance of fix 1

For contiguous out, or no out, the v?Conj call is unchanged and so is its cost. That is the only path the ASV benchmarks cover. For a non-contiguous out, the element-by-element conjugation costs about as much as NumPy's np.conjugate(view, out=view) on the same view: about 1.4–3.4 ms for 2^20 or 64³ elements. Master was cheaper there only because it conjugated the wrong memory. Strided v?ConjI calls were tried and gave no gain for 3-D views, where memory traffic sets the cost.

Tests

New tests are appended to test_fft1d.py and test_fftnd.py:

  • fft/ifft and fftn/ifftn of real input (float32 and float64) into strided or reversed out, checking that nothing outside out is written;
  • rfft/rfftn/rfft2 with Fortran-ordered out for int64, float16, longdouble and float64 input.
Build New tests (46) Full suite
master 17 fail, 2 crash with SIGSEGV —
this PR all pass (also over 15 repeated runs) 2671 passed, 2 skipped

The cases that pass on master are the forward transforms and the float64 / unaffected-axis controls.

There are no new compiler warnings. This PR is independent of #387 and #388; it may conflict with #387 only in CHANGELOG.md.

Two pre-existing bugs with a user-provided `out`:

- `ifft`/`ifftn` of real input compute a forward transform and then
  complex-conjugate the result with `v?Conj`, treating `out` as one
  contiguous block. When `out` was not contiguous (e.g. a strided or
  reversed view whose strides match the input), part of it was left
  unconjugated and memory outside of `out` was modified, up to heap
  corruption. Conjugate element-wise along the strides of `out` unless
  it is contiguous.

- `_r2c_fft1d_impl` decided whether MKL can write into `out` directly
  from the contiguity of the original `x` rather than of `x_arr`, which
  for dtypes other than float32/float64 is a C-contiguous float64 copy.
  With Fortran-ordered integer/float16/longdouble input and `out`,
  `rfft`/`rfftn` gave wrong results.
The cast of input with dtypes other than float32/float64 to float64 in
`_r2c_fft1d_impl` requested NPY_ARRAY_ENSURECOPY, which makes
PyArray_FROM_OTF return a C-contiguous copy. For Fortran-ordered input
this is a slow reordering copy, and with `out=` it also required a
C-ordered temporary that is then copied into the Fortran-ordered `out`.
The cast always copies anyway, so drop the flag: the copy keeps the
memory order of the input, as the cast in `_c2r_fft1d_impl` already does.

For Fortran-ordered int64 input of shape 128^3 this brings `rfft` from
~45 ms to ~5 ms, and the result layout now matches NumPy, which allocates
the output with `empty_like` and so keeps the memory order of the input.
@antonwolfy

Copy link
Copy Markdown
Collaborator Author

Performance results for this PR (head 4c71b70) against master (5bc05b5).

Setup. Intel Core Ultra 5 236V (Lunar Lake: 4 P-cores and 4 LP E-cores), Ubuntu 25.10, Python 3.13, NumPy 2.5.3, oneMKL 2026.2. Both builds were compiled from source with gcc 15.2 and pass the full test suite.

  • The CPU governor was set to performance.
  • Runs were pinned to P-cores: 1 thread on a single P-core, and 4 threads on all four.
  • Within each round the two builds ran back-to-back. The figures are medians over 3 rounds (2 for the conjugation benchmark).

1. rfft of cast input (NPY_ARRAY_ENSURECOPY dropped)

Times are for 1 thread; the last column gives the speedup for 1 and 4 threads.

128³ input master this PR speedup (1 / 4 threads)
rfft int64, F-order, axis 0, out= F-order 27.5 ms 3.4 ms 8.1x / 8.8x
rfft int64, F-order, axis 0 20.9 ms 3.4 ms 6.1x / 7.6x
rfft float16, F-order, axis 0, out= F-order 20.2 ms 3.3 ms 6.1x / 5.0x
rfftn int64, F-order 22.3 ms 13.1 ms 1.7x / 2.6x
rfft int64, strided C-order, out= C-order 4.0 ms 3.5 ms 1.15x / 1.25x
rfft float64, F-order (control, unchanged path) 2.03 ms 2.04 ms 1.00x / 0.93x

In the strided C-order case the peak temporary memory also drops from 32 to 16 MiB, because no extra temporary is allocated.

2. Real-input ifft / ifftn conjugation (C change)

The inverse transform of real input is a forward transform followed by an in-place conjugation. So the conjugation cost is measured as time(ifft) - time(fft) for the same input and out, timed alternately in the same process. Values are for 1 thread, with the total ifft time in parentheses.

case master this PR
contiguous out, 1-D 2^20 0.64 ms (6.03) 0.65 ms (6.02)
contiguous out, ifftn 64³ 0.19 ms (1.14) 0.09 ms (1.04)
out[::2], 1-D 2^20 0.94 ms (6.61) 2.35 ms (8.04)
64³ strided view, axis=0 0.22 ms (4.75) 1.25 ms (5.80)
64³ strided view, axis=2 0.18 ms (3.11) 1.10 ms (4.04)
64³ strided view, ifftn 0.25 ms (4.54) 1.16 ms (5.45)
  • Contiguous out: unchanged. It still uses the single v?Conj call.
  • Non-contiguous out: the conjugation now costs 1.1–2.9 ms, which is 20–36% of the ifft time, with 1 or 4 threads.
  • Why master looks cheaper: master's numbers for non-contiguous out aren't a valid baseline. It conjugated a contiguous span, which is the wrong elements.
  • Compared with NumPy: np.conjugate(view, out=view) on the same views takes 0.9 ms (64³) and 1.6 ms (2^20 [::2]). So the new loop matches NumPy for the 3-D views and is about 1.4x slower for the 1-D strided case.

3. Contiguous paths: no regression

These are 60 cases mirroring the ASV suite, all contiguous or with no out:

  • fft/ifft/rfft/irfft 1-D at n = 1024, 16384 and 2^20;
  • fftn/ifftn/rfftn at 64³ and 128³;
  • float32, float64, complex64 and complex128;
  • contiguous out= for real-input ifft/ifftn.

As a control, master was also compared with a byte-identical copy of itself:

1 thread, 60 cases median ratio range cases > 10% off
master copy vs master (A/A) 1.000 0.71–1.37 31
this PR vs master 0.995 0.74–1.33 34
this PR vs master copy 1.000 0.85–1.17 3

With 4 threads, the median ratio of this PR against master is 1.003.

The PR stays within the A/A noise. Individual cases vary by up to ±30% between processes even with identical binaries. This tracks the 64-byte alignment of the arrays: for example, fft of 1024 complex128 takes 1.9 µs with the output at offset 0 and 1.4 µs at offset 16 or 48. Single-case swings in ASV results for this PR should be read with that in mind.

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