Repository: cupy/cupy
Pull request: Add `out` arg verifier in new random interface.
Fixes #4903

BTW I want to rework the tests and add a common baseclass for distributions in the old and new API as most of them are 100% identical, right now we are duplicating code where we shouldnt.

Problem statement:
`out` arg of methods of `random.Generator` should be more strict
At least, NumPy checks dtype and contiguity of `out`.

* Conditions (you can just paste the output of `python -c 'import cupy; cupy.show_config()'`)
  - CuPy version: master
* Code to reproduce

```python
import traceback

import cupy
import numpy


for xp in (numpy, cupy):
    print(xp.__name__)
    rng = xp.random.default_rng(0)

    x = xp.empty(3, xp.float32)
    try:
        print(rng.standard_exponential(3, xp.float64, out=x))
    except Exception as e:
        print("".join(traceback.TracebackException.from_exception(e).format_exception_only()))

    x = xp.empty((4, 3), xp.float32)[::2]
    try:
        print(rng.standard_exponential((2, 3), xp.float64, out=x))
    except Exception as e:
        print("".join(traceback.TracebackException.from_exception(e).format_exception_only()))
```

* Error messages, stack traces, or logs

```
numpy
TypeError: Supplied output array has the wrong type. Expected float64, got float32

ValueError: Supplied output array is not contiguous, writable or aligned.

cupy
[0.01653717 2.41721892 3.52366781]
[[1.14068234 0.17892487 0.3184453 ]
 [0.23529601 0.59083831 1.9350549 ]]
```

Hints from issue discussion:

Commit message: Create a mixin for invalid outs tests

Patch to review:
diff --git a/cupy/random/_generator_api.pyx b/cupy/random/_generator_api.pyx
index 669cd7fbd20..2092a0e9aea 100644
--- a/cupy/random/_generator_api.pyx
+++ b/cupy/random/_generator_api.pyx
@@ -56,6 +56,27 @@ class Generator:
     def __init__(self, bit_generator):
         self.bit_generator = bit_generator
 
+    def _check_output_array(self, dtype, size, out):
+        # Checks borrowed from NumPy
+        # https://github.com/numpy/numpy/blob/cb557b79fa0ce467c881830f8e8e042c484ccfaa/numpy/random/_common.pyx#L235-L251
+        dtype = numpy.dtype(dtype)
+        if out.dtype.char != dtype.char:
+            raise TypeError(
+                f'Supplied output array has the wrong type. '
+                f'Expected {dtype.name}, got {out.dtype.name}')
+        if not (out.flags.c_contiguous or out.flags.f_contiguous):
+            raise ValueError(
+                'Supplied output array is not contiguous,'
+                ' writable or aligned.')
+        if size is not None:
+            try:
+                tup_size = tuple(size)
+            except TypeError:
+                tup_size = tuple([size])
+            if tup_size != out.shape:
+                raise ValueError(
+                    'size must match out.shape when used together')
+
     def integers(
             self, low, high=None, size=None,
             dtype=numpy.int64, endpoint=False):
@@ -185,6 +206,9 @@ class Generator:
         if method == 'zig':
             raise NotImplementedError('Ziggurat method is not supported')
 
+        if out is not None:
+            self._check_output_array(dtype, size, out)
+
         y = ndarray(size if size is not None else (), numpy.float64)
         _launch_dist(self.bit_generator, exponential, y, ())
         if out is not None:
diff --git a/tests/cupy_tests/random_tests/test_generator_api.py b/tests/cupy_tests/random_tests/test_generator_api.py
index 8292e715f3c..98cb5969614 100644
--- a/tests/cupy_tests/random_tests/test_generator_api.py
+++ b/tests/cupy_tests/random_tests/test_generator_api.py
@@ -206,10 +206,37 @@ def test_beta_ks(self, dtype):
             a=self.a, b=self.b, size=2000, dtype=dtype)
 
 
+class InvalidOutsMixin:
+
+    def invalid_dtype_out(self, **kwargs):
+        out = cupy.zeros((3, 2), dtype=cupy.float32)
+        with pytest.raises(TypeError):
+            self.generate(size=(3, 2), out=out, **kwargs)
+
+    def invalid_contiguity(self, **kwargs):
+        out = cupy.zeros((4, 6), dtype=cupy.float64)[0:3:, 0:2:]
+        with pytest.raises(ValueError):
+            self.generate(size=(3, 2), out=out, **kwargs)
+
+    def invalid_shape(self, **kwargs):
+        out = cupy.zeros((3, 3), dtype=cupy.float64)
+        with pytest.raises(ValueError):
+            self.generate(size=(3, 2), out=out, **kwargs)
+
+    def test_invalid_dtype_out(self):
+        self.invalid_dtype_out()
+
+    def test_invalid_contiguity(self):
+        self.invalid_contiguity()
+
+    def test_invalid_shape(self):
+        self.invalid_shape()
+
+
 @testing.with_requires('numpy>=1.17.0')
 @testing.gpu
 @testing.fix_random()
-class TestStandardExponential(GeneratorTestCase):
+class TestStandardExponential(GeneratorTestCase, InvalidOutsMixin):
 
     target_method = 'standard_exponential'
 


Code review comments:
diff_hunk: @@ -206,10 +206,37 @@ def test_beta_ks(self, dtype):
             a=self.a, b=self.b, size=2000, dtype=dtype)
 
 
+class InvalidOutsMixin:
+
+    def invalid_dtype_out(self, **kwargs):
+        out = cupy.zeros((3, 2), dtype=cupy.float32)
+        with pytest.raises(TypeError):
+            self.generate(size=(3, 2), out=out, **kwargs)
+
+    def invalid_contiguity(self, **kwargs):
+        out = cupy.zeros((4, 6), dtype=cupy.float64)[0:3:, 0:2:]
+        with pytest.raises(ValueError):
+            self.generate(size=(3, 2), out=out, **kwargs)
+
+    def invalid_shape(self, **kwargs):
+        out = cupy.zeros((3, 3), dtype=cupy.float64)
+        with pytest.raises(ValueError):
+            self.generate(size=(3, 2), out=out, **kwargs)
+
+    def test_invalid_dtype_out(self):
+        self.invalid_dtype_out()
+
+    def test_invalid_contiguity(self):
+        self.invalid_contiguity()
+
+    def test_invalid_shape(self):
+        self.invalid_shape()
+
+
 @testing.with_requires('numpy>=1.17.0')
 @testing.gpu
 @testing.fix_random()
-class TestStandardExponential(GeneratorTestCase):
+class TestStandardExponential(GeneratorTestCase, InvalidOutsMixin):
path: tests/cupy_tests/random_tests/test_generator_api.py
text: @user1:
```suggestion
class TestStandardExponential(InvalidOutsMixin, GeneratorTestCase):
```

In case the method names conflicts (between `InvalidOutsMixin` and one of base classes), it seems usual that mixin classes go left and a base class goes right.