Skip to content

perf(FourierUnit): single fp32 region — +8% fps, −20% VRAM, weight-compatible#1

Open
NevermindNilas wants to merge 2 commits into
enhancr:mainfrom
NevermindNilas:perf/fourierunit-fp32-region
Open

perf(FourierUnit): single fp32 region — +8% fps, −20% VRAM, weight-compatible#1
NevermindNilas wants to merge 2 commits into
enhancr:mainfrom
NevermindNilas:perf/fourierunit-fp32-region

Conversation

@NevermindNilas

Copy link
Copy Markdown

What

FourierUnit.forward runs the FFT block as a single fp32 region instead of casting the spectrum down to fp16 for the intervening norm/conv ops and back up for the inverse FFT.

The FFT mandates fp32. The old path did 4 casts per FourierUnit (fp32→fp16 around the convs, fp16→fp32 around the FFTs). Keeping the norm/convs in the same fp32 region needs only 2 casts (entry/exit), cutting memory traffic and peak VRAM.

Why it's safe (weight-compatible, no retrain)

  • Existing checkpoints load unchanged — no params added/removed/reshaped.
  • FP32 inference path is bit-identical to before.
  • FP16 path differs only by running the FFT-block convs in fp32, which is strictly more accurate.

Performance (RTX 3090, x4, autocast FP16, drift-canceled interleaved bench)

input fps peak VRAM
360p (640×360) 6.68 → 7.49 (+8.7%) 551 → 436 MB (−115)
480p (854×480) 3.84 → 4.13 (+7.7%) 892 → 686 MB (−206)

FP32: speed/VRAM neutral, output bit-identical. VRAM saving grows with resolution (larger spectrum tensors).

Quality (Urban100, 5 frames, x4, edited vs original, identical weights)

Verified on real periodic structure (building facades), where FFT ringing would surface — not synthetic noise.

PSNR vs fp32 GT SSIM
original arch fp16 (noise floor) 74.60 dB 0.99999
edited arch fp16 74.70 dB 0.99999

Δ +0.10 dB — within fp16 noise (in fact fractionally better, since the convs now run fp32). FP32 output bit-exact (PSNR ∞ / SSIM 1.000000). Edited meets-or-beats the noise floor on all 5 frames.

Notes

No torch.compile / CUDA graphs / channels_last — those were explicitly out of scope, and channels_last in particular measured −26% here because the FFT path forces NCHW and fights it. This is a pure arithmetic-region change.

🤖 Generated with Claude Code

The FFT mandates fp32. The previous forward cast the spectrum down to
fp16 for the intervening norm/conv ops and back up for the inverse FFT,
i.e. 4 casts per FourierUnit. Keeping those ops in the same fp32 region
needs only 2 casts (entry/exit), cutting memory traffic and peak VRAM.

Weight-compatible: existing checkpoints load unchanged. The FP32
inference path is bit-identical; the FP16 path differs only by running
the FFT-block convs in fp32, which is strictly more accurate.

Measured on RTX 3090, x4, autocast FP16, drift-canceled interleaved bench:
  360p (640x360):  6.68 -> 7.49 fps (+8.7%),  551 -> 436 MB (-115 MB)
  480p (854x480):  3.84 -> 4.13 fps (+7.7%),  892 -> 686 MB (-206 MB)
FP32: speed/VRAM neutral, output bit-identical.

Quality (Urban100, 5 frames, x4, edited vs original, identical weights):
  FP32: bit-exact (PSNR inf / SSIM 1.000000)
  FP16 vs fp32 ground truth: 74.70 dB / 0.99999 SSIM, vs the original
  arch's own fp16 noise floor of 74.60 dB / 0.99999 SSIM -> +0.10 dB.
  Within fp16 noise (verified on real periodic structure, not synthetic
  noise, where FFT ringing would surface).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Copilot AI review requested due to automatic review settings May 25, 2026 21:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR optimizes the FFT-based forward pass by keeping FFT-adjacent operations in fp32 to reduce dtype cast round-trips and memory traffic under mixed precision.

Changes:

  • Disable AMP autocast around the FFT block and run intermediate ops in fp32.
  • Replace contiguous().view(...) with reshape(...) after a permute(...).
  • Move final dtype restoration to a single cast at the end of forward.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread figsr_arch.py Outdated
Comment on lines +619 to +620
with torch.autocast("cuda", enabled=False):
x = x.float()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in cb01d85 — autocast now uses x.device.type. (Note: torch.autocast("cuda", enabled=False) is actually a no-op on CPU and doesn't raise — verified — but device-agnostic is cleaner for the export path.)

Comment thread figsr_arch.py Outdated
with torch.autocast("cuda", enabled=False):
x = x.float()
ffted = self.rfft2(x)
ffted = ffted.permute(0, 4, 1, 2, 3).reshape(b, c * 2, h, -1)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Made explicit in cb01d85 (contiguous().view(...)). For reference these are equivalent here — after a permute, reshape performs the same single copy as contiguous().view, so no perf difference — but explicit is fine.

Comment thread figsr_arch.py
Comment on lines +629 to +630
out = self.post_norm(out)
return out.to(orig_dtype)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Intentional and measured. post_norm is a custom RMSNorm (norm + addcmul), not a fused fp16 kernel — and AMP's autocast policy already runs normalization in fp32, so keeping it in this fp32 region matches prior numerics rather than changing them.

The suggested alternative (post_norm outside the disabled block) re-introduces a cast: irfft returns fp32, so post_norm in fp16 needs an extra fp32→fp16 round-trip — exactly the round-trip this PR removes (4 casts → 2). The +8% fps / −20% VRAM benchmark already has post_norm inside the region, so its effect is included and the net is positive. Moving it out measured slower.

Address Copilot review:
- autocast device_type from x.device.type instead of hard-coded "cuda"
  (CPU/export forward now uses the matching no-op autocast context).
- Make the post-rfft copy explicit (contiguous().view) instead of reshape.

Equivalence unchanged: fp32 bit-identical, fp16 maxdiff 6.5e-4.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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