Skip to content

Add norm parameter to FFT transforms (backward/ortho/forward)#3287

Merged
zcbenz merged 5 commits intoml-explore:mainfrom
Aristide021:codex/fft-norm
Mar 25, 2026
Merged

Add norm parameter to FFT transforms (backward/ortho/forward)#3287
zcbenz merged 5 commits intoml-explore:mainfrom
Aristide021:codex/fft-norm

Conversation

@Aristide021
Copy link
Copy Markdown
Contributor

Proposed changes

Adds norm support ("backward", "ortho", "forward") to FFT transforms for NumPy parity.

Matches NumPy numpy.fft norm conventions and addresses the normalization scaling gap noted in the mlx/primitives.cpp FFT VJP TODO.

  • Adds FFTNorm to core FFT API and threads it through fft/ifft/rfft/irfft, fft2/ifft2/rfft2/irfft2, fftn/ifftn/rfftn/irfftn
  • Implements normalization scaling in mlx/fft.cpp:
    • "backward": existing behavior (default)
    • "ortho": 1/sqrt(N) forward, sqrt(N) inverse adjustment
    • "forward": 1/N forward, N inverse adjustment
  • Adds Python norm="backward" parameter for all 12 FFT APIs with input validation
  • Adds NumPy parity tests for all norm modes and updates FFT docs

Default behavior is unchanged (norm="backward").

Checklist

  • I have read the CONTRIBUTING document
  • I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the necessary documentation (if needed)

Copy link
Copy Markdown
Collaborator

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

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

The StreamOrDevice parameter should always be put last.

@Aristide021
Copy link
Copy Markdown
Contributor Author

The StreamOrDevice parameter should always be put last.

Moved StreamOrDevice to last across all FFT APIs and call sites.

Copy link
Copy Markdown
Collaborator

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

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

Thanks for the contributions! This basically looks good to me, just a few more small requests.

@Aristide021
Copy link
Copy Markdown
Contributor Author

The test_vmap_masked_scatter failure appears unrelated to this PR,
it tests vmap masked scatter behavior which is not touched by these
changes. May be a pre-existing flaky test. @zcbenz

@zcbenz zcbenz merged commit 57c813f into ml-explore:main Mar 25, 2026
31 of 32 checks passed
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