Skip to content

BUG: unnecessary copy in .astype if the output array is already the… - #468

Merged
ev-br merged 1 commit into
data-apis:mainfrom
Nin17:fft-astype-copy
Aug 24, 2026
Merged

BUG: unnecessary copy in .astype if the output array is already the…#468
ev-br merged 1 commit into
data-apis:mainfrom
Nin17:fft-astype-copy

Conversation

@Nin17

@Nin17 Nin17 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

… correct dtype.

The fft functions unnecessarily copy the return array if the dtype is the desired dtype.
I believe the use of .astype is to enable support for numpy < 2.0. As support for "f4" and "c8" dtypes was introduced in numpy 2.0 the unnecessary copies only occur in numpy >= 2.0.
I'm not sure when cupy stopped upcasting "f4" and "c8" dtypes in ffts, but it currently doesn't, and therefore also has the unnecessary copy.

Copilot AI lite review requested due to automatic review settings August 23, 2026 11:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR avoids unnecessary array copies in FFT wrappers by passing copy=False to .astype(...) when enforcing expected output dtypes. This improves performance for backends (notably NumPy >= 2.0 and current CuPy) that already return the desired dtype.

Changes:

  • Update FFT wrapper functions to use res.astype(target_dtype, copy=False) when normalizing output dtypes for float32/complex64 inputs.
  • Update fftfreq/rfftfreq wrappers to use copy=False when an explicit dtype is requested.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Nin17

Nin17 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

The numpy test fails:
FAILED array_api_tests/test_linalg.py::test_trace
Seems unrelated to these changes.

@ev-br

ev-br commented Aug 23, 2026

Copy link
Copy Markdown
Member

CI failures are unrelated, data-apis/array-api-tests#455

@ev-br ev-br left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thank you @Nin17

@ev-br
ev-br merged commit 54e351c into data-apis:main Aug 24, 2026
26 of 28 checks passed
@ev-br ev-br added this to the 1.16 milestone Aug 24, 2026
@Nin17
Nin17 deleted the fft-astype-copy branch August 24, 2026 20:32
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.

3 participants