Repository: cupy/cupy
Pull request: Raise `IndexError` for R2C/C2R FFT with `axes=()`
Close #3240. This makes the behavior compatible with NumPy.

Problem statement:
Real FFTs with empty `axes` should fail
The shape of the output of `fft.(i)rfft*` depends on the last axis.
```
>>> numpy.fft.rfftn(numpy.arange(3), axes=())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<__array_function__ internals>", line 5, in rfftn
  File "/path/to/python3.8/site-packages/numpy-1.18.2-py3.8-linux-x86_64.egg/numpy/fft/_pocketfft.py", line 1124, in rfftn
    a = rfft(a, s[-1], axes[-1], norm)
IndexError: list index out of range
>>> cupy.fft.rfftn(cupy.arange(3), axes=())
array([0, 1, 2])
```

Hints from issue discussion:
Is this issue introduced in recent NumPy changes? Otherwise I guess it should it be considered as a bug  (cc/ #3241)
> Is this issue introduced in recent NumPy changes?

No.  The result with NumPy 1.15.4 is the same.


Should I fix this in #3102? I am touching rfft/irfft there.
> Should I fix this in #3102? I am touching rfft/irfft there.

Sounds good, ... only if #3102 already fixed it.

- For the `_fft` case, I suppose the fix will be independent.
- For the `_fftn` case, the addition of `value_type='C2C'` option to `_prep_fftn_axes` in #3102 looks good also in this sense.  However, in the current implementation, `_fft` is returned by `_default_fft_func` if < 2 dim.
@toslunar @grlee77 I am tracing back the cause of this issue, and I don't think https://github.com/cupy/cupy/pull/2746#issuecomment-563439184 also holds for C2R/R2C...It's only valid for C2C, so we need a conditional branch to fix this.

~~I think this is best addressed in the functions `_fft()` and `_fftn()`. Will fix the latter in #3102, and leave the former in a separate PR as @toslunar suggested.~~
Sorry, changed my mind. It's simpler to fix this in a standalone PR. See #3264.

Commit message: raise IndexError for R2C/C2R FFT with axes=()

Patch to review:
diff --git a/cupy/fft/fft.py b/cupy/fft/fft.py
index 6c9938ea851..d6ca3a1b3b2 100644
--- a/cupy/fft/fft.py
+++ b/cupy/fft/fft.py
@@ -166,8 +166,11 @@ def _fft(a, s, axes, norm, direction, value_type='C2C', overwrite_x=False,
         axes = [i for i in range(-dim, 0)]
     else:
         axes = tuple(axes)
-    if not axes:
-        return a
+    if len(axes) == 0:
+        if value_type == 'C2C':
+            return a
+        else:
+            raise IndexError('list index out of range')
     a = _convert_dtype(a, value_type)
     a = _cook_shape(a, s, axes, value_type)
 
@@ -419,8 +422,11 @@ def _fftn(a, s, axes, norm, direction, value_type='C2C', order='A', plan=None,
                          % norm)
 
     axes, axes_sorted = _prep_fftn_axes(a.ndim, s, axes)
-    if not axes_sorted:
-        return a
+    if len(axes_sorted) == 0:
+        if value_type == 'C2C':
+            return a
+        else:
+            raise IndexError('list index out of range')
     a = _convert_dtype(a, value_type)
 
     if order == 'A':
diff --git a/tests/cupy_tests/fft_tests/test_fft.py b/tests/cupy_tests/fft_tests/test_fft.py
index cc3fa3dd0d1..806c0116a7f 100644
--- a/tests/cupy_tests/fft_tests/test_fft.py
+++ b/tests/cupy_tests/fft_tests/test_fft.py
@@ -310,6 +310,7 @@ def test_fft_allocate(self):
     {'shape': (3, 4), 's': None, 'axes': (-1, -2), 'norm': None},
     {'shape': (3, 4), 's': None, 'axes': (0,), 'norm': None},
     {'shape': (3, 4), 's': None, 'axes': None, 'norm': 'ortho'},
+    {'shape': (3, 4), 's': None, 'axes': (), 'norm': None},
     {'shape': (2, 3, 4), 's': None, 'axes': None, 'norm': None},
     {'shape': (2, 3, 4), 's': (1, 4, None), 'axes': None, 'norm': None},
     {'shape': (2, 3, 4), 's': (1, 4, 10), 'axes': None, 'norm': None},
@@ -317,6 +318,7 @@ def test_fft_allocate(self):
     {'shape': (2, 3, 4), 's': None, 'axes': (-1, -2, -3), 'norm': None},
     {'shape': (2, 3, 4), 's': None, 'axes': (0, 1), 'norm': None},
     {'shape': (2, 3, 4), 's': None, 'axes': None, 'norm': 'ortho'},
+    {'shape': (2, 3, 4), 's': None, 'axes': (), 'norm': None},
     {'shape': (2, 3, 4), 's': (2, 3), 'axes': (0, 1, 2), 'norm': 'ortho'},
     {'shape': (2, 3, 4, 5), 's': None, 'axes': None, 'norm': None},
 )
@@ -370,6 +372,7 @@ def test_ifft2(self, xp, dtype, enable_nd):
     {'shape': (2, 3, 4), 's': None, 'axes': (-1, -3), 'norm': None},
     {'shape': (2, 3, 4), 's': None, 'axes': (0, 1), 'norm': None},
     {'shape': (2, 3, 4), 's': None, 'axes': None, 'norm': 'ortho'},
+    {'shape': (2, 3, 4), 's': None, 'axes': (), 'norm': 'ortho'},
     {'shape': (2, 3, 4), 's': (2, 3), 'axes': (0, 1, 2), 'norm': 'ortho'},
     {'shape': (2, 3, 4), 's': (4, 3, 2), 'axes': (2, 0, 1), 'norm': 'ortho'},
     {'shape': (2, 3, 4, 5), 's': None, 'axes': None, 'norm': None},
@@ -787,6 +790,28 @@ def test_irfft2(self, xp, dtype):
         return out
 
 
+@testing.parameterize(
+    {'shape': (3, 4), 's': None, 'axes': (), 'norm': None},
+    {'shape': (2, 3, 4), 's': None, 'axes': (), 'norm': None},
+)
+@testing.gpu
+class TestRfft2EmptyAxes(unittest.TestCase):
+
+    @testing.for_all_dtypes(no_complex=True)
+    def test_rfft2(self, dtype):
+        for xp in (np, cupy):
+            a = testing.shaped_random(self.shape, xp, dtype)
+            with pytest.raises(IndexError):
+                xp.fft.rfft2(a, s=self.s, axes=self.axes, norm=self.norm)
+
+    @testing.for_all_dtypes()
+    def test_irfft2(self, dtype):
+        for xp in (np, cupy):
+            a = testing.shaped_random(self.shape, xp, dtype)
+            with pytest.raises(IndexError):
+                xp.fft.irfft2(a, s=self.s, axes=self.axes, norm=self.norm)
+
+
 @testing.parameterize(
     {'shape': (3, 4), 's': None, 'axes': None, 'norm': None},
     {'shape': (3, 4), 's': (1, None), 'axes': None, 'norm': None},
@@ -839,6 +864,28 @@ def test_irfftn(self, xp, dtype):
         return out
 
 
+@testing.parameterize(
+    {'shape': (3, 4), 's': None, 'axes': (), 'norm': None},
+    {'shape': (2, 3, 4), 's': None, 'axes': (), 'norm': None},
+)
+@testing.gpu
+class TestRfftnEmptyAxes(unittest.TestCase):
+
+    @testing.for_all_dtypes(no_complex=True)
+    def test_rfftn(self, dtype):
+        for xp in (np, cupy):
+            a = testing.shaped_random(self.shape, xp, dtype)
+            with pytest.raises(IndexError):
+                xp.fft.rfftn(a, s=self.s, axes=self.axes, norm=self.norm)
+
+    @testing.for_all_dtypes()
+    def test_irfftn(self, dtype):
+        for xp in (np, cupy):
+            a = testing.shaped_random(self.shape, xp, dtype)
+            with pytest.raises(IndexError):
+                xp.fft.irfftn(a, s=self.s, axes=self.axes, norm=self.norm)
+
+
 @testing.parameterize(*testing.product({
     'n': [None, 5, 10, 15],
     'shape': [(10,), (10, 10)],


Code review comments:
diff_hunk: @@ -166,8 +166,11 @@ def _fft(a, s, axes, norm, direction, value_type='C2C', overwrite_x=False,
         axes = [i for i in range(-dim, 0)]
     else:
         axes = tuple(axes)
-    if not axes:
-        return a
+    if len(axes) == 0:
path: cupy/fft/fft.py
text: @user1:
Could you keep the original condition?  PEP8 says
> For sequences, (strings, lists, tuples), use the fact that empty sequences are false:

@author:
```suggestion
    if not axes:
```

diff_hunk: @@ -419,8 +422,11 @@ def _fftn(a, s, axes, norm, direction, value_type='C2C', order='A', plan=None,
                          % norm)
 
     axes, axes_sorted = _prep_fftn_axes(a.ndim, s, axes)
-    if not axes_sorted:
-        return a
+    if len(axes_sorted) == 0:
path: cupy/fft/fft.py
text: @user1:
ditto

@author:
```suggestion
    if not axes_sorted:
```