Repository: cupy/cupy
Pull request: `testing.numpy_cupy_allclose` with per-dtype tolerance
Closes #4220.

This PR makes `testing.numpy_cupy_allclose` allow tolerance per dtype.


Problem statement:
Proposal: Tolerance per dtype for `cupy.testing.numpy_cupy_allclose`
Currently, `cupy.testing.numpy_cupy_allclose` takes only a tolerance value, so we need to split a test or make an imperative call of `cupy.testing.assert_allclose` to supply different tolerance values per dtype.

Here proposed is making `cupy.testing.numpy_cupy_allclose` take association from a dtype to a tolerance value something as following:

```Python
@testing.numpy_cupy_allclose(rtol={np.float32: 1e-4, np.float64: 1e-12,
                                   np.complex64: 1e-4, np.complex128: 1e-12,
                                   'default': 1e-7},
                             atol={np.float32: 1e-4, np.float64: 1e-12,
                                   np.complex64: 1e-4, np.complex128: 1e-12,
                                   'default': 1e-7},
                             sp_name='sp')
```

Actual representation would have room for discussion. If the idea looks good to you, I will implement this feature.

Hints from issue discussion:

Commit message: Update docstring

Patch to review:
diff --git a/cupy/testing/helper.py b/cupy/testing/helper.py
index cbd6a03099a..d594e26970a 100644
--- a/cupy/testing/helper.py
+++ b/cupy/testing/helper.py
@@ -352,6 +352,33 @@ def _convert_output_to_ndarray(c_out, n_out, sp_name, check_sparse_format):
             type(c_out), type(n_out)))
 
 
+def _resolve_tolerance(type_check, result, rtol, atol):
+    def _resolve(dtype, tol):
+        if isinstance(tol, dict):
+            tol1 = tol.get(dtype.type)
+            if tol1 is None:
+                tol1 = tol.get('default')
+                if tol1 is None:
+                    raise TypeError(
+                        'Can not find tolerance for {}'.format(dtype.type))
+            return tol1
+        else:
+            return tol
+
+    # When `type_check` is `False`, cupy result and numpy result may have
+    # different dtypes so we can not determine the dtype to use from the
+    # tolerance associations.
+    if not type_check:
+        if isinstance(rtol, dict) or isinstance(atol, dict):
+            raise TypeError('When `type_ckeck` is `False`, `rtol` and `atol` '
+                            'must be supplied as float.')
+
+    dtype = result.dtype
+    rtol1 = _resolve(dtype, rtol)
+    atol1 = _resolve(dtype, atol)
+    return rtol1, atol1
+
+
 def numpy_cupy_allclose(rtol=1e-7, atol=0, err_msg='', verbose=True,
                         name='xp', type_check=True, accept_error=False,
                         sp_name=None, scipy_name=None, contiguous_check=True,
@@ -359,8 +386,13 @@ def numpy_cupy_allclose(rtol=1e-7, atol=0, err_msg='', verbose=True,
     """Decorator that checks NumPy results and CuPy ones are close.
 
     Args:
-         rtol(float): Relative tolerance.
-         atol(float): Absolute tolerance.
+         rtol(float or dict): Relative tolerance. Besides a float value, a
+             dictionary that maps a dtypes to a float value can be supplied to
+             adjust tolerance per dtype. If the dictionary has ``'default'``
+             string as its key, its value is used as the default tolerance in
+             case any dtype keys do not match.
+         atol(float or dict): Absolute tolerance. Besides a float value, a
+             dictionary can be supplied as ``rtol``.
          err_msg(str): The error message to be printed in case of failure.
          verbose(bool): If ``True``, the conflicting values are
              appended to the error message.
@@ -404,7 +436,8 @@ def numpy_cupy_allclose(rtol=1e-7, atol=0, err_msg='', verbose=True,
     .. seealso:: :func:`cupy.testing.assert_allclose`
     """
     def check_func(c, n):
-        array.assert_allclose(c, n, rtol, atol, err_msg, verbose)
+        rtol1, atol1 = _resolve_tolerance(type_check, c, rtol, atol)
+        array.assert_allclose(c, n, rtol1, atol1, err_msg, verbose)
     return _make_decorator(check_func, name, type_check, contiguous_check,
                            accept_error, sp_name, scipy_name,
                            _check_sparse_format)
diff --git a/tests/cupy_tests/testing_tests/test_helper.py b/tests/cupy_tests/testing_tests/test_helper.py
index 4417cf6b784..15d0df528be 100644
--- a/tests/cupy_tests/testing_tests/test_helper.py
+++ b/tests/cupy_tests/testing_tests/test_helper.py
@@ -145,6 +145,27 @@ def strange_kw_func(self, foo):
         return make_result(foo, numpy.array(2), cupy.array(1))
 
 
+class TestNumPyCuPyAllCloseTolPerDtype(unittest.TestCase):
+
+    @helper.for_float_dtypes()
+    @helper.numpy_cupy_allclose(rtol={numpy.float16: 1e-3, 'default': 1e-6})
+    def test_rtol_per_dtype(self, xp, dtype):
+        if xp is numpy:
+            return numpy.array(1, dtype=dtype)
+        else:
+            finfo = numpy.finfo(dtype)
+            return cupy.array(1 + finfo.eps, dtype=dtype)
+
+    @helper.for_float_dtypes()
+    @helper.numpy_cupy_allclose(atol={numpy.float16: 1e-3, 'default': 1e-6})
+    def test_atol_per_dtype(self, xp, dtype):
+        if xp is numpy:
+            return numpy.array(0, dtype=dtype)
+        else:
+            finfo = numpy.finfo(dtype)
+            return cupy.array(finfo.eps, dtype=dtype)
+
+
 class TestIgnoreOfNegativeValueDifferenceOnCpuAndGpu(unittest.TestCase):
 
     @helper.numpy_cupy_allclose()


Code review comments:
diff_hunk: @@ -352,15 +352,47 @@ def _convert_output_to_ndarray(c_out, n_out, sp_name, check_sparse_format):
             type(c_out), type(n_out)))
 
 
+def _resolve_tolerance(type_check, result, rtol, atol):
+    def _resolve(dtype, tol):
+        if isinstance(tol, dict):
+            tol1 = tol.get(dtype.type)
path: cupy/testing/helper.py
text: @user1:
If `tol` is specified with a wrong format, for example `{'float16': 1e-3}`, the default tolerance will be chosen and may pass the wrong test unexpectedly. Did you have some ideas to avoid this case?

@author:
Definitely. I think an option is using an association list `[(<dtype>, <tolerance>)]` instead of a dictionary to represent per-dtype tolerance, where `<dtype>` is an object such that `numpy.dtype(<dtype>) == {numpy,cupy}_result.dtype` is satisfied. This way we can use `[(numpy.float16, 1e-3)]`, `[('float16', 1e-3)]` or `[('f', 1e-3)]` as well. How do you think?

@user1:
Hmm, I feel the `dict` is a more natural interface. How about checking if all keys are of `type` type or `default` during the preprocess?

@author:
Sounds good. I'll fix the code as such.

@author:
I've made a commit, PTAL!