Fix RFFT crash on zero fft_length by changing validation from >= 0 to > 0 by YashSachdeva369 · Pull Request #123902 · tensorflow/tensorflow

YashSachdeva369 · 2026-07-24T09:08:30Z

Fixes #123399: tf.raw_ops.RFFT crashes with fatal signal on fft_length=[0]

Root cause:
The validation in FFTBase::Compute() and FFTNBase::Compute()
(tensorflow/core/kernels/fft_ops.cc) incorrectly accepts fft_length[i] == 0
via the >= 0 check. A zero fft_length flows into the FFT backend (DUCC for CPU,
cuFFT for GPU), which aborts with an internal assertion instead of TensorFlow
raising a clean InvalidArgumentError.

Fix:
Change both OP_REQUIRES checks from >= 0 to > 0 and update error messages
from "must >= 0" to "must be >= 1". This protects both CPU and GPU paths at once
since the check runs in the shared Compute() before dispatch.

Testing:
Added test case in fft_ops_test.py to verify fft_length=[0] now raises
InvalidArgumentError instead of crashing. Verified negative fft_length values
are still correctly rejected.

google-cla · 2026-07-24T09:08:41Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist

Code Review

This pull request updates TensorFlow's FFT kernels to ensure that all dimensions in fft_length are strictly greater than zero (>= 1), and updates the corresponding error messages and unit tests. The review feedback suggests using the public fft_ops.rfft wrapper instead of gen_spectral_ops.rfft in the new test case to maintain consistency with the rest of the test suite.

YashSachdeva369

update

keerthanakadiri · 2026-07-24T11:26:09Z

Hi @YashSachdeva369 , Can you kindly sign the CLA? Thank you!

dmiltr3

Thank you for the PR! The intention to fix the crash on fft_length=[0] is correct, but the current approach of changing the validation from >= 0 to > 0 causes regressions in handling empty tensors.

Problem

In TensorFlow, it is valid to process empty tensors (tensors where at least one dimension is 0). If an empty tensor is passed to RFFT (or related ops) without an explicit fft_length, the length is inferred from the inner dimensions of the input. If one of those dimensions is 0, the inferred fft_length will contain 0.

By changing the validation to > 0, the kernel now rejects these valid inferred zero lengths with an InvalidArgumentError (e.g., fft_length[0] must be >= 1, but got: 0), breaking existing workflows and tests that rely on empty tensor propagation (such as test_empty in signal tests).

Suggested Alternatives

Option 1: Early Exit (Safe and Simple)

Instead of blocking 0 in validation, you can keep the >= 0 check and handle the zero length safely during kernel execution. If the computed output_shape has zero elements (or input_shape is empty), you can early exit before dispatching to the backend (DUCC/cuFFT). This prevents the backend crash while preserving support for empty tensor inputs.

In FFTBase::Compute and FFTNBase::Compute:

// After computing output_shape
if (output_shape.num_elements() == 0 || input_shape.num_elements() == 0) {
  // Return early, avoiding backend call with invalid dimensions
  return;
}

Note: This will cause fft_length=0 on non-empty inputs to return an empty tensor safely, which might differ slightly from NumPy's rfft behavior (which returns length 1 for n=0), but it is consistent with TensorFlow's empty tensor semantics and avoids the crash.

Option 2: Conditional Validation

If the goal is to strictly reject explicit zero lengths for non-empty inputs (because the backend doesn't support them well), the validation should be conditioned on the input shape.

// Only reject 0 if the input is NOT empty
OP_REQUIRES(ctx, fft_length_as_vec(i) > 0 || input_shape.num_elements() == 0,
            absl::InvalidArgumentError(absl::StrCat(
                "fft_length[", i, "] must be > 0 for non-empty inputs, but got: ",
                fft_length_as_vec(i))));

Tests

If you choose Option 1, you can verify it works correctly with empty inputs and explicit zeros by running standard signal tests.

Here is a simple test case you could add to fft_ops_test.py to verify that explicit zero lengths do not crash (if you pursue Option 1):

def test_zero_fft_length_non_empty_input(self):
    # This test verifies that zero fft_length does not crash on non-empty input.
    x = tf.constant([1.0, 2.0, 3.0], dtype=tf.float32)
    # Should yield empty tensor or correct shape safely
    y = tf.raw_ops.RFFT(input=x, fft_length=[0])
    self.assertEqual(y.shape, [0])

If you pursue Option 2, you can assert that InvalidArgumentError is raised only for non-empty inputs:

def test_zero_fft_length_non_empty_input_fails(self):
    x = tf.constant([1.0, 2.0, 3.0], dtype=tf.float32)
    with self.assertRaises(tf.errors.InvalidArgumentError):
        tf.raw_ops.RFFT(input=x, fft_length=[0])

YashSachdeva369 · 2026-07-28T07:08:13Z

The remaining Windows failure appears unrelated to this FFT change. The failing target is //tensorflow/cc:framework_cc_ops_test, which fails while linking because Abseil Cord symbols (for example, CrcCordState) are unresolved. The Linux CPU, CUDA, CUDA 13, and ARM64 checks passed. Could a maintainer please rerun or investigate the Windows toolchain/dependency failure?

dmiltr3

Feedback & Required Fixes

We found a couple of issues in the implementation of _validate_nonnegative_fft_length in fft_ops.py that cause test failures (specifically TypeError in regression tests).

1. Robustness to Multi-dimensional Arrays

In _validate_nonnegative_fft_length, np.atleast_1d is used to make fft_length iterable. However, if fft_length is already multi-dimensional (e.g. 2D array [[0]]), np.atleast_1d leaves it as 2D.
When you iterate over a 2D array in Python, you get 1D arrays (rows).
Calling int(fft_length_i) on a 1D array yields:
TypeError: only 0-dimensional arrays can be converted to Python scalars

Fix: Flatten the array before iterating to ensure you are checking scalars.

2. Preserving Type/Shape

Returning fft_length_static_array (which is always at least 1D array) changes the type of fft_length in the wrapper if it was originally passed as a scalar.
In the original code, fft_length was updated with whatever constant_value returned, preserving its shape (scalar vs array).

Fix: Return the original fft_length_static instead of the converted array.

Suggested Implementation for `_validate_nonnegative_fft_length`:

def _validate_nonnegative_fft_length(fft_length):
  """Raises if a statically known FFT length is negative."""
  fft_length_static = _tensor_util.constant_value(fft_length)
  if fft_length_static is None:
    return None
  # Flatten to handle multi-dimensional arrays robustly.
  for fft_length_i in np.asarray(fft_length_static).flat:
    if int(fft_length_i) < 0:
      raise ValueError(
          f"`fft_length` must be non-negative, got {fft_length_i}."
      )
  return fft_length_static

YashSachdeva369 · 2026-08-02T10:15:01Z

Thanks for the review. I updated _validate_nonnegative_fft_length in fft_ops.py so it:

flattens fft_length_static with np.asarray(...).flat,
checks every scalar value for negativity,
and returns the original fft_length_static to preserve scalar vs array shape.
I also kept the regression coverage in fft_ops_test.py.

Please let me know if you want an extra test for the multi-dimensional fft_length case.

YashSachdeva369 · 2026-08-11T05:48:03Z

Hi @dmiltr3 @gbaned, thanks again for the review and approval! I noticed the import/copybara check is currently failing with "An error happened while migrating the change," while all the actual CI builds (Linux CPU, CUDA, CUDA13, ARM64, etc.) are passing. Since this looks like it's on the internal import side rather than something in the PR itself, could someone please retry the Copybara import when you get a chance? Let me know if there's anything on my end I should adjust. Thanks!

nithyak0204 · 2026-08-12T04:51:31Z

Hi @YashSachdeva369, Can you please resolve the conflicts? Thank you!

dmiltr3

CI Test Failures in `fft_ops_test`

CI checks on Linux CPU, CUDA, CUDA 13, and Windows x86 failed due to regression test failures in //tensorflow/python/kernel_tests/signal:fft_ops_test_cpu:

Failure Details

The following existing test cases failed:

testIRFFTZeroOrNegativeLengthRaisesError
testIRFFTNDZeroOrNegativeLengthRaisesError

AssertionError: "fft_length\[-1\] must be > 0" does not match "`fft_length` must be non-negative, got -5."

Root Cause

_validate_nonnegative_fft_length was called inside _irfft and _irfftn in tensorflow/python/ops/signal/fft_ops.py.

For inverse real FFT operations (irfft, irfft2d, irfft3d, irfftn), the last dimension requirement is stricter (fft_length[-1] > 0) and is already validated by _validate_static_irfft_fft_length(fft_length_static, fft_rank) on non-empty inputs. Calling _validate_nonnegative_fft_length in _irfft / _irfftn intercepts negative lengths and raises ValueError: `fft_length` must be non-negative, got ... before _validate_static_irfft_fft_length can produce the expected error message (fft_length[-1] must be > 0).

Required Fix

Remove the _validate_nonnegative_fft_length calls from _irfft and _irfftn in tensorflow/python/ops/signal/fft_ops.py. Keep _validate_nonnegative_fft_length in _rfft, _fftn, _ifftn, and _rfftn.

diff --git a/tensorflow/python/ops/signal/fft_ops.py b/tensorflow/python/ops/signal/fft_ops.py
index 9e3752ea16..7553b6cb65 100644
--- a/tensorflow/python/ops/signal/fft_ops.py
+++ b/tensorflow/python/ops/signal/fft_ops.py
@@ -219,7 +219,6 @@ def _irfft_wrapper(ifft_fn, fft_rank, default_name):
         fft_length = _infer_fft_length_for_irfft(input_tensor, fft_rank)
       else:
         fft_length = _ops.convert_to_tensor(fft_length, _dtypes.int32)
-      fft_length_static = _validate_nonnegative_fft_length(fft_length)
       fft_length_static = _tensor_util.constant_value(fft_length)
       is_empty = input_tensor.shape.num_elements() == 0
       if not is_empty:
@@ -401,7 +400,6 @@ def _irfftn_wrapper(irfft_n, default_name):
         fft_length = _infer_fft_length_for_irfftn(input_tensor)
       else:
         fft_length = _ops.convert_to_tensor(fft_length, _dtypes.int32)
-      fft_length_static = _validate_nonnegative_fft_length(fft_length)
       fft_length_static = _tensor_util.constant_value(fft_length)
       if input_tensor.shape.num_elements() != 0:
         _validate_static_irfft_fft_length(fft_length_static, fft_rank)

YashSachdeva369 · 2026-08-14T09:04:57Z

@dmiltr3 i change all the things as you said. Please review it.

dmiltr3

The changes look good and address all previous review comments. All test cases and CI checks pass cleanly.

…from >= 0 to > 0 Imported from GitHub PR tensorflow/tensorflow#123902 Fixes #123399: tf.raw_ops.RFFT crashes with fatal signal on fft_length=[0] **Root cause:** The validation in FFTBase::Compute() and FFTNBase::Compute() (tensorflow/core/kernels/fft_ops.cc) incorrectly accepts fft_length[i] == 0 via the `>= 0` check. A zero fft_length flows into the FFT backend (DUCC for CPU, cuFFT for GPU), which aborts with an internal assertion instead of TensorFlow raising a clean InvalidArgumentError. **Fix:** Change both OP_REQUIRES checks from `>= 0` to `> 0` and update error messages from "must >= 0" to "must be >= 1". This protects both CPU and GPU paths at once since the check runs in the shared Compute() before dispatch. **Testing:** Added test case in fft_ops_test.py to verify fft_length=[0] now raises InvalidArgumentError instead of crashing. Verified negative fft_length values are still correctly rejected. Copybara import of the project: -- ed14b79b74e5e37c5b71c006eed8d21006e8aafd by Yash Sachdeva <wecanexplore369@gmail.com>: Fix zero fft length validation -- ace0879fd4cb9e7f51d828da5cf6164359001d76 by Yash Sachdeva <wecanexplore369@gmail.com>: Handle zero FFT lengths without backend dispatch -- 9026c120108849c7c79450f5f66f6c06c893ae13 by Yash Sachdeva <wecanexplore369@gmail.com>: Preserve static FFT length shape during validation -- e586bff34cf21c10f8156447bc3fb0b3ce979ee1 by Yash Sachdeva <wecanexplore369@gmail.com>: Fix FFT length validation for scalar and multidimensional values -- fb5b61870fc561952691ce816a5db11122d79a73 by Yash Sachdeva <wecanexplore369@gmail.com>: Fix FFT length validation regex and flatten constant fft_length values for scalar/multi-dimensional cases -- e6807c7664ffed932aeb5b39a7f4b8fc6f7d702a by Yash Sachdeva <wecanexplore369@gmail.com>: Fix duplicated trailing block in tensorflow/python/kernel_tests/signal/fft_ops_test.py -- 1f8f54e8c95922088de928bab976fd840988c365 by Yash Sachdeva <wecanexplore369@gmail.com>: Fix Windows _pywrap_tensorflow LNK2019 for ConverterPassOptionsSetter::SetOptions Add a minimal, Windows-only provider cc_library (converter_pass_options_setter_provider) in mlir/lite that force-links the ConverterPassOptionsSetter vtable and SetOptions overloads via alwayslink=1. The provider is added to the _pywrap_tensorflow deps gated with if_windows() so Linux/macOS link closures are unchanged, avoiding the CI regressions from the naive unconditional dependency. -- 7e4609f332b8879ba0d95001c43064d8e7090886 by Yash Sachdeva <wecanexplore369@gmail.com>: Add common and converter_flags_proto_cc deps to converter_pass_options_setter_provider -- e0d6e943b384a42016557841206576a5dc466832 by Yash Sachdeva <wecanexplore369@gmail.com>: Pin actions/checkout to SHA in gemini.yml to satisfy GitHub Actions Scan -- 680f8ee9e8cce0a2c9f4e919176897103c72f27f by Yash Sachdeva <wecanexplore369@gmail.com>: Fix IRFFT validation order for negative lengths Merging this change closes #123902 FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#123902 from YashSachdeva369:fix-rfft-zero-fft-length 680f8ee9e8cce0a2c9f4e919176897103c72f27f LiteRT-Converter-PiperOrigin-RevId: 964664985

YashSachdeva369 · 2026-08-14T14:45:50Z

How to resolve this issue so that other checks don't get affected?

dmiltr3

Summary of Required Changes

Revert Unrelated Changes:
The PR includes commits modifying files outside the scope of the FFT fix:
- .github/workflows/gemini.yml
- tensorflow/compiler/mlir/lite/BUILD
- tensorflow/compiler/mlir/lite/transforms/converter_pass_options_setter_provider.cc
- tensorflow/python/BUILD
These additions were made to address an external Windows build issue, but they introduce out-of-scope code into MLIR Lite / Python build rules and cause automated import/sync failures. Please revert changes to these 4 files so the PR strictly contains the FFT fix.

Restore Deleted Test Assertion in fft_ops_test.py:
In tensorflow/python/kernel_tests/signal/fft_ops_test.py, the assertion comparing fftshift with axes=-1 vs axes=(1,) in test_negative_axes was inadvertently deleted:

@@ -1045,6 +1045,8 @@ class FFTShiftTest(test.TestCase, parameterized.TestCase):
       shifted = [[-1, -3, -2], [2, 0, 1], [-4, 3, 4]]
       self.assertAllEqual(fft_ops.fftshift(freqs, axes=(0, -1)), shifted)
       self.assertAllEqual(fft_ops.ifftshift(shifted, axes=(0, -1)), freqs)
+      self.assertAllEqual(
+          fft_ops.fftshift(freqs, axes=-1), fft_ops.fftshift(freqs, axes=(1,)))
       self.assertAllEqual(
           fft_ops.ifftshift(shifted, axes=-1),
           fft_ops.ifftshift(shifted, axes=(1,)))

CI Status Note:
The failure in build-windows-x86 (//tensorflow/c:c_test) is an unrelated linker/symbol issue on the Windows runner and will be addressed separately on master. Once the unrelated files are reverted and the test assertion is restored, the PR will be ready to merge.

YashSachdeva369 · 2026-08-15T08:06:12Z

@dmiltr3 all checks are passed. Please review it.

Fix zero fft length validation

ed14b79

google-ml-butler Bot added the size:S CL Change Size: Small label Jul 24, 2026

google-ml-butler Bot assigned gbaned Jul 24, 2026

gemini-code-assist Bot reviewed Jul 24, 2026

View reviewed changes

Comment thread tensorflow/python/kernel_tests/signal/fft_ops_test.py Outdated

YashSachdeva369 commented Jul 24, 2026

View reviewed changes

keerthanakadiri requested a review from a team July 24, 2026 11:25

google-ml-butler Bot added the awaiting review Pull request awaiting review label Jul 24, 2026

keerthanakadiri added this to PR Queue Jul 24, 2026

github-project-automation Bot moved this to Assigned Reviewer in PR Queue Jul 24, 2026

keerthanakadiri added comp:core issues related to core part of tensorflow prtype:bugfix PR to fix a bug labels Jul 24, 2026

dmiltr3 requested changes Jul 27, 2026

View reviewed changes

github-project-automation Bot moved this from Assigned Reviewer to Reviewer Requested Changes in PR Queue Jul 27, 2026

YashSachdeva369 added 2 commits July 27, 2026 11:26

Handle zero FFT lengths without backend dispatch

ace0879

Handle zero FFT lengths without backend dispatch

0a776ca

YashSachdeva369 force-pushed the fix-rfft-zero-fft-length branch from ed14b79 to 0a776ca Compare July 27, 2026 05:58

Merge remote branch and update FFT validation test

6a21f24

nithyak0204 requested a review from dmiltr3 July 28, 2026 09:25

dmiltr3 requested changes Jul 28, 2026

View reviewed changes

Preserve static FFT length shape during validation

9026c12

nithyak0204 added stat:awaiting response Status - Awaiting response from author and removed awaiting review Pull request awaiting review labels Jul 29, 2026

Fix FFT length validation for scalar and multidimensional values

e586bff

google-ml-butler Bot removed the stat:awaiting response Status - Awaiting response from author label Aug 2, 2026

dmiltr3 self-requested a review August 2, 2026 19:26

google-ml-butler Bot added the awaiting review Pull request awaiting review label Aug 2, 2026

github-project-automation Bot moved this from Reviewer Requested Changes to Approved by Reviewer in PR Queue Aug 10, 2026

kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 10, 2026

nithyak0204 added stat:awaiting response Status - Awaiting response from author and removed ready to pull PR ready for merge process labels Aug 12, 2026

Merge branch 'master' into fix-rfft-zero-fft-length

1902c6b

YashSachdeva369 dismissed dmiltr3’s stale review via 1902c6b August 12, 2026 06:18

dmiltr3 requested changes Aug 12, 2026

View reviewed changes

github-project-automation Bot moved this from Approved by Reviewer to Reviewer Requested Changes in PR Queue Aug 12, 2026

Fix IRFFT validation order for negative lengths

680f8ee

google-ml-butler Bot removed the stat:awaiting response Status - Awaiting response from author label Aug 14, 2026

dmiltr3 previously approved these changes Aug 14, 2026

View reviewed changes

google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Aug 14, 2026

github-project-automation Bot moved this from Reviewer Requested Changes to Approved by Reviewer in PR Queue Aug 14, 2026

copybara-service Bot mentioned this pull request Aug 14, 2026

PR #123902: Fix RFFT crash on zero fft_length by changing validation from >= 0 to > 0 google-ai-edge/LiteRT#9346

Open

kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 14, 2026

Fix fft_ops function spacing

af456ad

YashSachdeva369 dismissed dmiltr3’s stale review via af456ad August 14, 2026 22:37

google-ml-butler Bot removed the ready to pull PR ready for merge process label Aug 14, 2026

dmiltr3 requested changes Aug 15, 2026

View reviewed changes

github-project-automation Bot moved this from Approved by Reviewer to Reviewer Requested Changes in PR Queue Aug 15, 2026

Limit FFT fix to relevant changes

7a2157a

Uh oh!

Conversation

YashSachdeva369 commented Jul 24, 2026

Uh oh!

google-cla Bot commented Jul 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

YashSachdeva369 left a comment

Choose a reason for hiding this comment

Uh oh!

keerthanakadiri commented Jul 24, 2026

Uh oh!

dmiltr3 left a comment

Choose a reason for hiding this comment

Problem

Suggested Alternatives

Option 1: Early Exit (Safe and Simple)

Option 2: Conditional Validation

Tests

Uh oh!

YashSachdeva369 commented Jul 28, 2026

Uh oh!

dmiltr3 left a comment

Choose a reason for hiding this comment

Feedback & Required Fixes

1. Robustness to Multi-dimensional Arrays

2. Preserving Type/Shape

Suggested Implementation for _validate_nonnegative_fft_length:

Uh oh!

YashSachdeva369 commented Aug 2, 2026

Uh oh!

YashSachdeva369 commented Aug 11, 2026

Uh oh!

nithyak0204 commented Aug 12, 2026

Uh oh!

dmiltr3 left a comment

Choose a reason for hiding this comment

CI Test Failures in fft_ops_test

Failure Details

Root Cause

Required Fix

Uh oh!

YashSachdeva369 commented Aug 14, 2026

Uh oh!

dmiltr3 left a comment

Choose a reason for hiding this comment

Uh oh!

YashSachdeva369 commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmiltr3 left a comment

Choose a reason for hiding this comment

Summary of Required Changes

Uh oh!

YashSachdeva369 commented Aug 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Suggested Implementation for `_validate_nonnegative_fft_length`:

CI Test Failures in `fft_ops_test`

YashSachdeva369 commented Aug 14, 2026 •

edited

Loading