Skip to content

Disallow minlength=None in dpnp.bincount #2310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions dpnp/dpnp_iface_histograms.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,17 +225,23 @@ def _get_bin_edges(a, bins, range, usm_type):


def _bincount_validate(x, weights, minlength):
dpnp.check_supported_arrays_type(x)
if x.ndim > 1:
raise ValueError("object too deep for desired array")

if x.ndim < 1:
raise ValueError("object of too small depth for desired array")

if not dpnp.issubdtype(x.dtype, dpnp.integer) and not dpnp.issubdtype(
x.dtype, dpnp.bool
):
raise TypeError("x must be an integer array")

if weights is not None:
dpnp.check_supported_arrays_type(weights)
if x.shape != weights.shape:
raise ValueError("The weights and x don't have the same length.")

if not (
dpnp.issubdtype(weights.dtype, dpnp.integer)
or dpnp.issubdtype(weights.dtype, dpnp.floating)
Expand All @@ -245,10 +251,12 @@ def _bincount_validate(x, weights, minlength):
f"Weights must be integer or float. Got {weights.dtype}"
)

if minlength is not None:
minlength = int(minlength)
if minlength < 0:
raise ValueError("minlength must be non-negative")
if minlength is None:
raise TypeError("use 0 instead of None for minlength")

minlength = int(minlength)
if minlength < 0:
raise ValueError("minlength must be non-negative")


def _bincount_run_native(
Expand All @@ -262,9 +270,7 @@ def _bincount_run_native(
if min_v < 0:
raise ValueError("x argument must have no negative arguments")

size = int(dpnp.max(max_v)) + 1
if minlength is not None:
size = max(size, minlength)
size = max(int(max_v) + 1, minlength)

# bincount implementation uses atomics, but atomics doesn't work with
# host usm memory
Expand Down Expand Up @@ -299,9 +305,9 @@ def _bincount_run_native(
return n_casted


def bincount(x, weights=None, minlength=None):
def bincount(x, weights=None, minlength=0):
"""
bincount(x, /, weights=None, minlength=None)
bincount(x, /, weights=None, minlength=0)

Count number of occurrences of each value in array of non-negative ints.

Expand All @@ -313,10 +319,12 @@ def bincount(x, weights=None, minlength=None):
Input 1-dimensional array with non-negative integer values.
weights : {None, dpnp.ndarray, usm_ndarray}, optional
Weights, array of the same shape as `x`.

Default: ``None``
minlength : {None, int}, optional
minlength : int, optional
A minimum number of bins for the output array.
Default: ``None``

Default: ``0``

Returns
-------
Expand Down Expand Up @@ -416,6 +424,7 @@ def digitize(x, bins, right=False):
increasing or decreasing.
right : bool, optional
Indicates whether the intervals include the right or the left bin edge.

Default: ``False``.

Returns
Expand Down Expand Up @@ -675,6 +684,7 @@ def histogram_bin_edges(a, bins=10, range=None, weights=None):
given range.
If `bins` is a sequence, it defines the 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
Expand All @@ -683,12 +693,14 @@ def histogram_bin_edges(a, bins=10, range=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``.
weights : {None, dpnp.ndarray, usm_ndarray}, optional
An array of weights, of the same shape as `a`. Each value in `a` only
contributes its associated weight towards the bin count (instead of 1).
This is currently not used by any of the bin estimators, but may be in
the future.

Default: ``None``.

Returns
Expand Down
7 changes: 4 additions & 3 deletions dpnp/tests/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
from numpy.testing import assert_allclose, assert_array_equal

import dpnp
from dpnp.tests import config

from . import config


def assert_dtype_allclose(
Expand Down Expand Up @@ -86,14 +87,14 @@ def assert_dtype_allclose(
assert dpnp_arr.dtype == numpy_arr.dtype


def get_integer_dtypes(no_unsigned=False):
def get_integer_dtypes(all_int_types=False, no_unsigned=False):
"""
Build a list of integer types supported by DPNP.
"""

dtypes = [dpnp.int32, dpnp.int64]

if config.all_int_types:
if config.all_int_types or all_int_types:
dtypes += [dpnp.int8, dpnp.int16]
if not no_unsigned:
dtypes += [dpnp.uint8, dpnp.uint16, dpnp.uint32, dpnp.uint64]
Expand Down
56 changes: 47 additions & 9 deletions dpnp/tests/test_histogram.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
assert_array_equal,
assert_raises,
assert_raises_regex,
suppress_warnings,
)

import dpnp
Expand All @@ -19,6 +18,7 @@
get_float_dtypes,
get_integer_dtypes,
has_support_aspect64,
numpy_version,
)


Expand Down Expand Up @@ -282,9 +282,10 @@ def test_weights(self, density):
assert_dtype_allclose(result_hist, expected_hist)
assert_dtype_allclose(result_edges, expected_edges)

def test_integer_weights(self):
@pytest.mark.parametrize("dt", get_integer_dtypes(all_int_types=True))
def test_integer_weights(self, dt):
v = numpy.array([1, 2, 2, 4])
w = numpy.array([4, 3, 2, 1])
w = numpy.array([4, 3, 2, 1], dtype=dt)

iv = dpnp.array(v)
iw = dpnp.array(w)
Expand Down Expand Up @@ -602,19 +603,42 @@ def test_different_bins_amount(self, bins_count):
@pytest.mark.parametrize(
"array",
[[1, 2, 3], [1, 2, 2, 1, 2, 4], [2, 2, 2, 2]],
ids=["[1, 2, 3]", "[1, 2, 2, 1, 2, 4]", "[2, 2, 2, 2]"],
ids=["size=3", "size=6", "size=4"],
)
@pytest.mark.parametrize(
"minlength", [0, 1, 3, 5], ids=["0", "1", "3", "5"]
)
def test_bincount_minlength(self, array, minlength):
@pytest.mark.parametrize("minlength", [0, 1, 3, 5])
def test_minlength(self, array, minlength):
np_a = numpy.array(array)
dpnp_a = dpnp.array(array)

expected = numpy.bincount(np_a, minlength=minlength)
result = dpnp.bincount(dpnp_a, minlength=minlength)
assert_allclose(expected, result)

@pytest.mark.filterwarnings("ignore::DeprecationWarning")
@pytest.mark.parametrize(
"xp",
[
dpnp,
pytest.param(
numpy,
marks=pytest.mark.xfail(
numpy_version() < "2.3.0",
reason="numpy deprecates but accepts that",
strict=True,
),
),
],
)
def test_minlength_none(self, xp):
a = xp.array([1, 2, 3])
assert_raises_regex(
TypeError,
"use 0 instead of None for minlength",
xp.bincount,
a,
minlength=None,
)

@pytest.mark.parametrize(
"array", [[1, 2, 2, 1, 2, 4]], ids=["[1, 2, 2, 1, 2, 4]"]
)
Expand All @@ -623,7 +647,7 @@ def test_bincount_minlength(self, array, minlength):
[None, [0.3, 0.5, 0.2, 0.7, 1.0, -0.6], [2, 2, 2, 2, 2, 2]],
ids=["None", "[0.3, 0.5, 0.2, 0.7, 1., -0.6]", "[2, 2, 2, 2, 2, 2]"],
)
def test_bincount_weights(self, array, weights):
def test_weights(self, array, weights):
np_a = numpy.array(array)
np_weights = numpy.array(weights) if weights is not None else weights
dpnp_a = dpnp.array(array)
Expand All @@ -633,6 +657,20 @@ def test_bincount_weights(self, array, weights):
result = dpnp.bincount(dpnp_a, weights=dpnp_weights)
assert_allclose(expected, result)

@pytest.mark.parametrize(
"data",
[numpy.arange(5), 3, [2, 1]],
ids=["numpy.ndarray", "scalar", "list"],
)
def test_unsupported_data_weights(self, data):
# check input array
msg = "An array must be any of supported type"
assert_raises_regex(TypeError, msg, dpnp.bincount, data)

# check array of weights
a = dpnp.ones(5, dtype=dpnp.int32)
assert_raises_regex(TypeError, msg, dpnp.bincount, a, weights=data)


class TestHistogramDd:
@pytest.mark.usefixtures("suppress_complex_warning")
Expand Down
Loading