Apply review comments

Author: AlexanderKalistratov

dpnp/dpnp_iface_histograms.py

@@ -42,15 +42,18 @@
 
 import dpctl.utils as dpu
 import numpy
-from dpctl.tensor._type_utils import _can_cast
 
 import dpnp
 
 # pylint: disable=no-name-in-module
 import dpnp.backend.extensions.statistics._statistics_impl as statistics_ext
+from dpnp.dpnp_utils.dpnp_utils_common import (
+    result_type_for_device,
+    to_supported_dtypes,
+)
 
 # pylint: disable=no-name-in-module
-from .dpnp_utils import get_usm_allocations, map_dtype_to_device
+from .dpnp_utils import get_usm_allocations
 
 __all__ = [
     "bincount",
@@ -66,33 +69,15 @@
 _range = range
 
 
-def _result_type_for_device(dtypes, device):
-    rt = dpnp.result_type(*dtypes)
-    return map_dtype_to_device(rt, device)
-
-
 def _align_dtypes(a_dtype, bins_dtype, ntype, supported_types, device):
-    has_fp64 = device.has_aspect_fp64
-    has_fp16 = device.has_aspect_fp16
-
-    a_bin_dtype = _result_type_for_device([a_dtype, bins_dtype], device)
+    a_bin_dtype = result_type_for_device([a_dtype, bins_dtype], device)
 
     # histogram implementation doesn't support uint64 as histogram type
     # we can use int64 instead. Result would be correct even in case of overflow
     if ntype == numpy.uint64:
         ntype = dpnp.int64
 
-    if (a_bin_dtype, ntype) in supported_types:
-        return a_bin_dtype, ntype
-
-    for sample_type, hist_type in supported_types:
-        if _can_cast(
-            a_bin_dtype, sample_type, has_fp16, has_fp64
-        ) and _can_cast(ntype, hist_type, has_fp16, has_fp64):
-            return sample_type, hist_type
-
-    # should not happen
-    return None, None
+    return to_supported_dtypes([a_bin_dtype, ntype], supported_types, device)
 
 
 def _ravel_check_a_and_weights(a, weights):
@@ -524,6 +509,7 @@ def histogram(a, bins=10, range=None, density=None, weights=None):
         If `bins` is a sequence, it defines a monotonically increasing array
         of bin edges, including the rightmost edge, allowing for non-uniform
         bin widths.
+
         Default: ``10``.
     range : {None, 2-tuple of float}, optional
         The lower and upper range of the bins. If not provided, range is simply
@@ -532,6 +518,7 @@ def histogram(a, bins=10, range=None, density=None, weights=None):
         affects the automatic bin computation as well. While bin width is
         computed to be optimal based on the actual data within `range`, the bin
         count will fill the entire range including portions containing no data.
+
         Default: ``None``.
     density : {None, bool}, optional
         If ``False`` or ``None``, the result will contain the number of samples
@@ -540,6 +527,7 @@ def histogram(a, bins=10, range=None, density=None, weights=None):
         the range is ``1``. Note that the sum of the histogram values will not
         be equal to ``1`` unless bins of unity width are chosen; it is not
         a probability *mass* function.
+
         Default: ``None``.
     weights : {None, dpnp.ndarray, usm_ndarray}, optional
         An array of weights, of the same shape as `a`. Each value in `a` only
@@ -549,6 +537,7 @@ def histogram(a, bins=10, range=None, density=None, weights=None):
         Please note that the ``dtype`` of `weights` will also become the
         ``dtype`` of the returned accumulator (`hist`), so it must be large
         enough to hold accumulated values as well.
+
         Default: ``None``.
 
     Returns
@@ -782,29 +771,31 @@ def histogram2d(x, y, bins=10, range=None, density=None, weights=None):
         * A combination [int, array] or [array, int], where int
           is the number of bins and array is the bin edges.
 
-        Default: ``10``
+        Default: ``10``.
     range : {None, dpnp.ndarray, usm_ndarray} of shape (2,2), optional
         The leftmost and rightmost edges of the bins along each dimension
-        (if not specified explicitly in the `bins` parameters):
-        ``[[xmin, xmax], [ymin, ymax]]``. All values outside of this range
-        will be considered outliers and not tallied in the histogram.
+        If ``None`` the ranges are
+        ``[[x.min(), x.max()], [y.min(), y.max()]]``. All values outside
+        of this range will be considered outliers and not tallied in the
+        histogram.
 
-        Default: ``None``
+        Default: ``None``.
     density : {None, bool}, optional
         If ``False`` or ``None``, the default, returns the number of
         samples in each bin.
         If ``True``, returns the probability *density* function at the bin,
         ``bin_count / sample_count / bin_volume``.
 
-        Default: ``None``
+        Default: ``None``.
     weights : {None, dpnp.ndarray, usm_ndarray} of shape (N,), optional
         An array of values ``w_i`` weighing each sample ``(x_i, y_i)``.
         Weights are normalized to ``1`` if `density` is ``True``.
         If `density` is ``False``, the values of the returned histogram
         are equal to the sum of the weights belonging to the samples
         falling into each bin.
+        If ``None`` all samples are assigned a weight of ``1``.
 
-        Default: ``None``
+        Default: ``None``.
     Returns
     -------
     H : dpnp.ndarray of shape (nx, ny)
@@ -836,18 +827,27 @@ def histogram2d(x, y, bins=10, range=None, density=None, weights=None):
     Examples
     --------
     >>> import dpnp as np
-    >>> x = np.random.randn(20)
-    >>> y = np.random.randn(20)
+    >>> x = np.random.randn(20).astype("float32")
+    >>> y = np.random.randn(20).astype("float32")
     >>> hist, edges_x, edges_y = np.histogram2d(x, y, bins=(4, 3))
+    >>> hist.shape
+    (4, 3)
     >>> hist
-    [[1. 0. 0.]
-     [0. 0. 0.]
-     [5. 6. 4.]
-     [1. 2. 1.]]
+    array([[1., 2., 0.],
+           [0., 3., 1.],
+           [1., 4., 1.],
+           [1., 3., 3.]], dtype=float32)
+    >>> edges_x.shape
+    (5,)
     >>> edges_x
-    [-5.6575713 -3.5574734 -1.4573755  0.6427226  2.74282  ]
+    array([-1.7516936 , -0.96109843, -0.17050326,  0.62009203,  1.4106871 ],
+          dtype=float32)
+    >>> edges_y.shape
+    (4,)
     >>> edges_y
-    [-1.1889046  -0.07263839  1.0436279   2.159894  ]
+    array([-2.6604428 , -0.94615364,  0.76813555,  2.4824247 ], dtype=float32)
+
+    Please note, that resulting values of histogram and edges would be different
     """
 
     dpnp.check_supported_arrays_type(x, y)
@@ -869,7 +869,7 @@ def histogram2d(x, y, bins=10, range=None, density=None, weights=None):
     usm_type, exec_q = get_usm_allocations([x, y, bins, range, weights])
     device = exec_q.sycl_device
 
-    sample_dtype = _result_type_for_device([x.dtype, y.dtype], device)
+    sample_dtype = result_type_for_device([x.dtype, y.dtype], device)
 
     # Unlike histogramdd histogram2d accepts 1d bins and
     # apply it to both dimensions
@@ -883,7 +883,7 @@ def histogram2d(x, y, bins=10, range=None, density=None, weights=None):
     bins_dtypes = [sample_dtype]
     bins_dtypes += [b.dtype for b in bins if hasattr(b, "dtype")]
 
-    bins_dtype = _result_type_for_device(bins_dtypes, device)
+    bins_dtype = result_type_for_device(bins_dtypes, device)
     hist_dtype = _histdd_hist_dtype(exec_q, weights)
 
     supported_types = statistics_ext.histogramdd_dtypes()
@@ -1026,9 +1026,7 @@ def _histdd_hist_dtype(queue, weights):
         # hist_dtype is either float or complex, so it is ok
         # to calculate it as result type between default_float and
         # weights.dtype
-        hist_dtype = _result_type_for_device(
-            [hist_dtype, weights.dtype], device
-        )
+        hist_dtype = result_type_for_device([hist_dtype, weights.dtype], device)
 
     return hist_dtype
 
@@ -1039,7 +1037,7 @@ def _histdd_sample_dtype(queue, sample, bin_edges_list):
     dtypes_ = [bin_edges.dtype for bin_edges in bin_edges_list]
     dtypes_.append(sample.dtype)
 
-    return _result_type_for_device(dtypes_, device)
+    return result_type_for_device(dtypes_, device)
 
 
 def _histdd_supported_dtypes(sample, bin_edges_list, weights):
@@ -1089,31 +1087,33 @@ def histogramdd(sample, bins=10, range=None, density=None, weights=None):
         * The number of bins for each dimension (nx, ny, ... =bins)
         * The number of bins for all dimensions (nx=ny=...=bins).
 
-        Default: ``10``
+        Default: ``10``.
     range : {None, sequence}, optional
         A sequence of length D, each an optional (lower, upper) tuple giving
         the outer bin edges to be used if the edges are not given explicitly in
         `bins`.
-        An entry of None in the sequence results in the minimum and maximum
+        An entry of ``None`` in the sequence results in the minimum and maximum
         values being used for the corresponding dimension.
-        None is equivalent to passing a tuple of D None values.
+        ``None`` is equivalent to passing a tuple of D ``None`` values.
 
-        Default: ``None``
+        Default: ``None``.
     density : {None, bool}, optional
         If ``False`` or ``None``, the default, returns the number of
         samples in each bin.
         If ``True``, returns the probability *density* function at the bin,
         ``bin_count / sample_count / bin_volume``.
 
-        Default: ``None``
-    weights : {dpnp.ndarray, usm_ndarray}, optional
+        Default: ``None``.
+    weights : {None, dpnp.ndarray, usm_ndarray}, optional
         An (N,)-shaped array of values `w_i` weighing each sample
         `(x_i, y_i, z_i, ...)`.
-        Weights are normalized to 1 if density is True. If density is False,
-        the values of the returned histogram are equal to the sum of the
-        weights belonging to the samples falling into each bin.
+        Weights are normalized to ``1`` if density is ``True``.
+        If density is ``False``, the values of the returned histogram
+        are equal to the sum of the weights belonging to the samples
+        falling into each bin.
+        If ``None`` all samples are assigned a weight of ``1``.
 
-        Default: ``None``
+        Default: ``None``.
 
     Returns
     -------

dpnp/dpnp_utils/dpnp_utils_common.py

@@ -54,6 +54,19 @@ def to_supported_dtypes(dtypes, supported_types, device):
     def is_castable(dtype, stype):
         return _can_cast(dtype, stype, has_fp16, has_fp64)
 
+    if not isinstance(supported_types, Iterable):
+        supported_types = (supported_types,)
+
+    if isinstance(dtypes, Iterable):
+        sdtypes_elem = supported_types[0]
+        if not isinstance(sdtypes_elem, Iterable):
+            raise ValueError(
+                "Input and supported types must have the same length"
+            )
+
+        typ = type(sdtypes_elem)
+        dtypes = typ(dtypes)
+
     if dtypes in supported_types:
         return dtypes
 
@@ -78,4 +91,7 @@ def is_castable(dtype, stype):
             ):
                 return stypes
 
-    return None
+    if not isinstance(dtypes, Iterable):
+        return None
+
+    return (None,) * len(dtypes)

dpnp/tests/test_histogram.py

@@ -930,7 +930,7 @@ def test_2d(self):
     def test_finite_range(self, xp):
         x = y = xp.linspace(0.0, 1.0, num=100)
 
-        # normal ranges should be fine
+        # normal ranges should be finite
         _, _, _ = xp.histogram2d(x, y, range=[[0.25, 0.75]] * 2)
         assert_raises(
             ValueError, xp.histogram2d, x, y, range=[[xp.nan, 0.75]] * 2