Skip to content

perf: check x_arr, not x, when deciding whether irfft can write into out directly - #396

Open
intel-python-devops wants to merge 2 commits into
masterfrom
chore/agentic-sweep-22
Open

intel-python-devops wants to merge 2 commits into
masterfrom
chore/agentic-sweep-22

Conversation

@intel-python-devops

Copy link
Copy Markdown

Note

This pull request is entirely AI-generated. Please review thoroughly.

  • mkl_fft/_pydfti.pyx: In _c2r_fft1d_impl's out-of-place branch, the c_contig/f_contig heuristic that decides whether out can be written to directly (versus allocating a scratch buffer and copying) now checks x_arr.flags instead of x.flags.

x_arr is the array actually handed to the MKL backend (it may be a freshly-cast copy of x when the input dtype is real, e.g. for irfft on a float array), while x is the original user-supplied object; when x is non-contiguous but the cast copy x_arr happens to be C- or F-contiguous, checking x.flags wrongly concludes the layouts are incompatible and forces an avoidable scratch allocation plus a final out[...] = f_arr copy on every such call. The sibling function _r2c_fft1d_impl already fixed this exact pattern (its comment explicitly says "Check x_arr, which may be a cast copy of x with a different layout"); _c2r_fft1d_impl had been left checking x. The backend's irfft kernel (mklfft.c.src) always derives input and output DFTI strides independently from the actual x_arr/out arrays it is given, so this heuristic only ever gates whether the extra copy happens — never correctness — making the fix behaviour-preserving while enabling the zero-copy path in more cases.

@antonwolfy antonwolfy self-assigned this Oct 20, 2026
@antonwolfy antonwolfy added this to the 2.5.0 release milestone Oct 20, 2026
@antonwolfy antonwolfy changed the title perf: use the processed array's layout, not the original's, when decidin perf: check x_arr, not x, when deciding whether irfft can write into out directly Oct 20, 2026
…array

Add a test for `irfft` with `out=` on strided input that is cast (integer
and real dtypes) or zero-padded (complex dtype) into a contiguous copy,
in C and F order, which exercises MKL writing directly into `out`.

Add a changelog entry for gh-396.
@antonwolfy

Copy link
Copy Markdown
Collaborator

What this change does

In the out-of-place branch of _c2r_fft1d_impl, the check that decides whether MKL can write the result straight into out now looks at x_arr, the array actually passed to MKL, instead of the original x. x_arr differs from x when:

  • the input has a real or integer dtype and is cast to complex;
  • the input is zero-padded because n // 2 + 1 > x.shape[axis];
  • irfftn is called along a single axis: its last step calls _c2r_fft1d_impl on the (possibly trimmed) input.

When x is not contiguous but x_arr is, and out has the same memory order as x_arr, the result is now written into out directly instead of into a temporary array that is then copied into out.

A correction to the description: the check does not only decide whether an extra copy happens. For input with 3 or more dimensions, the backend makes a single batched MKL call based on the layout of the input alone, so the check is also what keeps input and output layouts that don't match away from that call. The same change in _r2c_fft1d_impl (#389) fixed wrong results. For irfft the old check happened to give correct results anyway, because the cast keeps the memory order of the input and padding keeps C/F order. So this PR is a performance change only, and the new check is the correct condition.

Measurements

Lunar Lake, irfft with out=, median in ms, master vs this PR:

input master this PR peak memory
2-D float64, strided, C-ordered out 3.55 1.18 16 → 8 MiB
2-D float32, strided, C-ordered out 1.04 0.62 8 → 4 MiB
3-D float64, strided, axis -1, C-ordered out 1.83 1.19 16 → 8 MiB
3-D float64, strided, axis 0, F-ordered out 1.83 1.19 16 → 8 MiB
2-D complex128, strided + padded, C-ordered out 1.93 1.29 16 → 8 MiB
2-D complex128, contiguous (path unchanged) 0.43 0.44 —

A check against numpy.fft over 1420 cases (irfft/irfftn; 5 dtypes; C, F, strided and permuted input; n that truncates or pads; C, F and strided out) passes before and after the change.

One case gets slower: input that is both C- and F-contiguous, such as shape (m, 1) zero-padded along axis 1, with a C-ordered out (0.21 → 0.52 ms for (4096, 1) padded to 64). _pad_array pads such input into an F-ordered copy, which no longer matches a C-ordered out. #397 fixes this in _pad_array (0.18 ms, and the result layout then matches NumPy); the two PRs merge without conflicts.

Pushed to this branch

16f2734 adds test_irfft_out_strided_input_copied, which covers the newly enabled direct path (strided cast and padded input, C- and F-ordered out, all axes), and a changelog entry.

The failed jenkins-build-wheel-mkl_fft-3.14t-linux-dev run is an infrastructure failure (the autolin04 agent kept going offline and the job lost its heartbeat) and needs a rerun.

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.

2 participants