Skip to content

core/ops/fft: Fft pulsification on non-FFT axes + guards#2261

Open
JulienBalianSonos wants to merge 1 commit into
mainfrom
feat/fft-pulse-non-fft-axis
Open

core/ops/fft: Fft pulsification on non-FFT axes + guards#2261
JulienBalianSonos wants to merge 1 commit into
mainfrom
feat/fft-pulse-non-fft-axis

Conversation

@JulienBalianSonos
Copy link
Copy Markdown
Collaborator

@JulienBalianSonos JulienBalianSonos commented May 20, 2026

Summary

Two commits on this branch.

1. Fft::axes_mapping = natural. Fft is rank-preserving and 1-to-1 on every axis (only the values along the FFT axis change), so the generic per-pulse wrapper can handle streaming on any non-FFT axis once axes_mapping reports the 1-to-1 relationship. Without this, pulse-pass bails on No specific pulse transformation for Fft, and could not track pulsing axis, which blocks STFT-based streaming because STFT pulse-lowers to a streaming-axis frame loop with a per-frame Fft running on a different axis (the frequency axis).

2. fft_pulsify: reject streaming on the FFT axis itself or on the trailing (re, imag) axis. The natural mapping would otherwise let PulseWrappingOp happily run Fft per-pulse on the FFT axis. That's a coherent operation: it's equivalent to Stft { frame: pulse_size, stride: pulse_size, axis } where the runtime picks pulse_size. But that is not the typed-model semantics the user wrote (Fft { axis } means "FFT every sample on that axis", which requires the axis length to be known and is undefined when the axis is symbolic). Silently reinterpreting it would be a foot-gun, so the pulsifier bails with a message that points users at Stft { frame: N, stride: N } and asks them to pick N explicitly. The complex-axis case is already rejected upstream in Fft::output_facts (trailing dim must be 2); the test locks in that earlier-layer rejection.

Edge-case sweep in harness/core-proptest-pulse/src/fft.rs:

  • Smoke + proptest for forward FFT on rank-4 (batch, stream, fft, 2).
  • Smoke + proptest mirror for inverse FFT.
  • Rank-3 (no batch) and rank-5 (extra leading dims, matches DPDFNet df_op shape) smoke cases.
  • Stacked FFT→iFFT roundtrip on the same axis (streaming traverses both via the natural mapping).
  • Negative paths: streaming on the FFT axis bails cleanly; streaming on the complex axis is rejected at typed-model build.

Status

Originally opened with just commit 1, self-closed pending the edge-case work the comment thread called out. Re-opened now with commit 2 covering the guards and the wider test sweep.

@JulienBalianSonos JulienBalianSonos changed the title core/ops/fft: add Fft::axes_mapping for pulse on non-FFT axis core/ops/fft: Fft pulsification on non-FFT axes + guards Jun 2, 2026
@JulienBalianSonos JulienBalianSonos requested a review from kali June 2, 2026 15:02
@kali
Copy link
Copy Markdown
Collaborator

kali commented Jun 5, 2026

Did you try just implementing axes_mapping ? Not that the propose implementation is incorrect. It can't be ::natural() the time axis and (and the complex axis) do not map.

I think with the map right, there should not be any need for dedicated pulsification.

@JulienBalianSonos JulienBalianSonos force-pushed the feat/fft-pulse-non-fft-axis branch from b0f810d to 431b5cf Compare June 5, 2026 11:46
@JulienBalianSonos
Copy link
Copy Markdown
Collaborator Author

@kali I tried to fix according to your remark to make it more principled. let me know

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