Repository navigation
fix: cast unsupported-dtype input to complex128 when out= is given - #387
intel-python-devops wants to merge 3 commits into
Conversation
`_c2c_fft1d_impl` decided whether `out` can be written directly by MKL by comparing its strides with the original `x`, but MKL reads `x_arr`, which may be a cast or zero-padded contiguous copy of `x`. When `x` and `out` shared a non-contiguous or F-ordered layout, rank >= 3 transforms gave wrong results and wrote outside of `out`. With the cast now also done when `out` is given, this affected integer/bool/float16 input as well as the pre-existing case of padded complex input. Also restore the original dtype-dispatch structure and apply the `out` override after the cast, as `_direct_fftnd` does; raise instead of silently returning when the out-of-place dispatch sees an unsupported type; move the regression tests into test_fft1d.py/test_fftnd.py with coverage for strided, F-ordered, truncated and padded layouts and for fftn/ifftn; and add CHANGELOG entries.
|
Pushed 2417932 on top of the original fix. Fixes
Cleanup
Results for the 74 new tests: 58 fail on master and 18 fail on the previous PR head (all of them strided or F-ordered cases). On this commit they all pass, as does the full suite. |
This comment was marked as resolved.
This comment was marked as resolved.
out= is giveout= is given
|
Performance results for this PR (head 70166a9) against master (e06bd74). 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 (2625 passed on master, 2699 here, which adds the new tests).
Summary. The paths that already worked are unchanged. 1. Zero-padded input with
|
fft(x, n=..., out=out) |
master | this PR | speedup, 1 thread | master | this PR | speedup, 4 threads |
|---|---|---|---|---|---|---|
| 256×1000 complex128, n=1024, last axis | 905 µs | 572 µs | 1.58x | 718 µs | 363 µs | 1.98x |
| 256×1000 float64, n=1024, last axis | 741 µs | 518 µs | 1.43x | 593 µs | 334 µs | 1.78x |
| 64×64×50 complex128, n=64, last axis | 719 µs | 492 µs | 1.46x | 674 µs | 361 µs | 1.86x |
| 1000×256 complex128, n=1024, axis 0 | 1548 µs | 1553 µs | 1.00x | 786 µs | 777 µs | 1.01x |
The axis-0 case is unchanged because the strides of x and out already matched on master. The speedup does not depend on array alignment.
2. Paths that already worked: no change
These are 23 cases: 1-D fft/ifft for complex128, complex64 and float32 input (n = 1024 to 2^20), with contiguous out and without out; 3-D 64³ on axes 0 and 2; complex input with a strided out of the same layout; and fft(x) / o[...] = fft(x) for int32, bool and float16 input.
| geometric mean of the per-case ratios | 4 threads | 1 thread |
|---|---|---|
| this PR / master | 0.982 | 1.001 |
| master copy / master (A/A) | 0.982 | 1.001 |
The PR is exactly as far from master as an identical copy of master is. Comparing only processes whose arrays landed on the same 64-byte alignment, this PR against master gives 0.995 (4 threads) and 1.001 (1 thread).
3. Cast path with out=: now works, cost in line with the work-arounds
On master fft(x, out=o) returned o unmodified for these dtypes, so the comparison is against what users do instead. Medians for int64 input at 1 thread / 4 threads (int32, bool and float16 behave the same within noise):
| int64 input | fft(x, out=o) (this PR) |
master: o[...] = fft(x) |
master: fft(x) |
np.fft.fft(x, out=o) |
|---|---|---|---|---|
| 65536 | 0.23 / 0.14 ms | 0.23 / 0.25 ms | 0.20 / 0.15 ms | 0.60 / 1.00 ms |
| 2^20 | 8.8 / 5.0 ms | 8.4 / 5.8 ms | 7.9 / 5.0 ms | 23.4 / 20.1 ms |
| 64³ | 0.43 / 0.28 ms | 0.51 / 0.57 ms | 0.31 / 0.25 ms | 0.79 / 0.89 ms |
- Compared with
o[...] = fft(x): at 1 thread it is within about ±15% (4% slower at 2^20, 16% faster at 64³); at 4 threads it is 15–50% faster, about 2x for the 64³ array. fft(x)withoutoutis somewhat faster because it transforms the converted copy in place, but it has noout.- It is about 2–7x faster than
np.fft.fft(x, out=o). - Peak memory is the same as the work-arounds.
- With a strided
out(a 64³ view), the result goes through a temporary and is copied, so that case takes about 4–11% longer thanview[...] = fft(x)and peaks at 8 MiB instead of 4 MiB. Before the stride fix in 2417932, this case wrote wrong values and wrote outsideout. - These cast-path timings are medians over 3 rounds. They are not controlled for alignment and carry about ±15% noise.
A note on measurement noise
In a first 4-thread run, some unchanged cases looked 20–70% slower with this PR, for example fft of n=16384 complex64 at 25 µs versus 14 µs. That turned out to be array alignment, not the code: at the same alignment both builds take about 24 µs at 16/48 bytes and 14–15 µs at 0/32 bytes, and the identical master copy reproduced the same gaps. So single-case swings of ±30% in ASV or Jenkins benchmark results for this PR should not be read as regressions without checking alignment.
_c2c_fft1d_implreturnedoutunmodified whenout=was given and the input dtype was not float32/float64/complex64/complex128 (e.g. integer, bool, float16). The cast to complex128 was skipped wheneveroutwas passed, so nothing was dispatched to MKL and the zerostatuswas reported as success.Affected entry points:
mkl_fft.fft/ifftmkl_fft.fftn/ifftn/fft2/ifft2(via_iter_fftnd)mkl_fft.interfaces.numpy_fftand patchednumpy.fftmkl_fft.interfaces.scipy_fftis not affected.Changes in
mkl_fft/_pydfti.pyx:out is not Noneforces out-of-place afterwards, as in_direct_fftnd.outis now decided by comparingout's strides withx_arr(the cast or padded copy that MKL reads) instead of the originalx. Comparing againstxgave wrong results and writes outsideoutfor rank >= 3 input whenxandoutshared a non-contiguous or F-ordered layout. This affected the newly enabled cast path and, already on master, complex input withn=zero-padding.ValueErrorinstead of returning silently.Behaviour change: on platforms where long double is wider than double, longdouble/clongdouble input with
out=now raisesValueError, as it already did withoutout, instead of returningoutunmodified.Tests (in
test_fft1d.pyandtest_fftnd.py):out=, includingntruncation and paddingout, checking that nothing outsideoutis writtenValueErrorfftn/ifftnwith integer inputCHANGELOG entries added under
[dev]/ Fixed.