From 9aa184235409e24a26dc2aac5c4ba9a6bc0db748 Mon Sep 17 00:00:00 2001 From: Nikita Grigorian Date: Wed, 23 Oct 2024 11:50:46 -0700 Subject: [PATCH 1/5] Improve `dpctl.tensor.full` error when `fill_value` is of an invalid type pybind11 raises RuntimeError when a value cannot be cast to a type, i.e., a string to an integer. Now when scalar is not an array, a check is performed, and TypeError is raised --- dpctl/tensor/_ctors.py | 6 ++++++ dpctl/tests/test_usm_ndarray_ctor.py | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/dpctl/tensor/_ctors.py b/dpctl/tensor/_ctors.py index d3d8fa64f5..f33b5de214 100644 --- a/dpctl/tensor/_ctors.py +++ b/dpctl/tensor/_ctors.py @@ -15,6 +15,7 @@ # limitations under the License. import operator +from numbers import Number import numpy as np @@ -1113,6 +1114,11 @@ def full( sycl_queue = normalize_queue_device(sycl_queue=sycl_queue, device=device) usm_type = usm_type if usm_type is not None else "device" + if not isinstance(fill_value, Number): + raise TypeError( + "`full` array cannot be constructed with value of type " + f"{type(fill_value)}" + ) dtype = _get_dtype(dtype, sycl_queue, ref_type=type(fill_value)) res = dpt.usm_ndarray( shape, diff --git a/dpctl/tests/test_usm_ndarray_ctor.py b/dpctl/tests/test_usm_ndarray_ctor.py index 046a2d9496..479bdc43eb 100644 --- a/dpctl/tests/test_usm_ndarray_ctor.py +++ b/dpctl/tests/test_usm_ndarray_ctor.py @@ -2621,3 +2621,10 @@ def test_setitem_from_numpy_contig(): expected = dpt.reshape(dpt.arange(-10, 10, dtype=fp_dt), (4, 5)) assert dpt.all(dpt.flip(Xdpt, axis=-1) == expected) + + +def test_full_raises_type_error(): + get_queue_or_skip() + + with pytest.raises(TypeError): + dpt.full(1, "0") From 31cb820ae02dfbf8bca2c0ed5b2f48fa134f39e5 Mon Sep 17 00:00:00 2001 From: Nikita Grigorian Date: Wed, 23 Oct 2024 12:10:33 -0700 Subject: [PATCH 2/5] Rearrange `fill_value` type check logic and implement in `full_like` --- dpctl/tensor/_ctors.py | 13 +++++++++---- dpctl/tests/test_usm_ndarray_ctor.py | 6 +++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/dpctl/tensor/_ctors.py b/dpctl/tensor/_ctors.py index f33b5de214..824655073c 100644 --- a/dpctl/tensor/_ctors.py +++ b/dpctl/tensor/_ctors.py @@ -1111,14 +1111,14 @@ def full( sycl_queue=sycl_queue, ) return dpt.copy(dpt.broadcast_to(X, shape), order=order) - - sycl_queue = normalize_queue_device(sycl_queue=sycl_queue, device=device) - usm_type = usm_type if usm_type is not None else "device" - if not isinstance(fill_value, Number): + elif not isinstance(fill_value, Number): raise TypeError( "`full` array cannot be constructed with value of type " f"{type(fill_value)}" ) + + sycl_queue = normalize_queue_device(sycl_queue=sycl_queue, device=device) + usm_type = usm_type if usm_type is not None else "device" dtype = _get_dtype(dtype, sycl_queue, ref_type=type(fill_value)) res = dpt.usm_ndarray( shape, @@ -1486,6 +1486,11 @@ def full_like( ) _manager.add_event_pair(hev, copy_ev) return res + elif not isinstance(fill_value, Number): + raise TypeError( + "`full` array cannot be constructed with value of type " + f"{type(fill_value)}" + ) dtype = _get_dtype(dtype, sycl_queue, ref_type=type(fill_value)) res = _empty_like_orderK(x, dtype, usm_type, sycl_queue) diff --git a/dpctl/tests/test_usm_ndarray_ctor.py b/dpctl/tests/test_usm_ndarray_ctor.py index 479bdc43eb..777a46f090 100644 --- a/dpctl/tests/test_usm_ndarray_ctor.py +++ b/dpctl/tests/test_usm_ndarray_ctor.py @@ -2623,8 +2623,12 @@ def test_setitem_from_numpy_contig(): assert dpt.all(dpt.flip(Xdpt, axis=-1) == expected) -def test_full_raises_type_error(): +def test_full_functions_raise_type_error(): get_queue_or_skip() with pytest.raises(TypeError): dpt.full(1, "0") + + x = dpt.ones(1, dtype="i4") + with pytest.raises(TypeError): + dpt.full_like(x, "0") From 748fde5370e98296f2023c0d138ebd5ab298011d Mon Sep 17 00:00:00 2001 From: Nikita Grigorian Date: Wed, 23 Oct 2024 12:14:57 -0700 Subject: [PATCH 3/5] Record change to `full` and `full_like` in changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50c6bcd926..615a99be5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +* Improved error in constructors `tensor.full` and `tensor.full_like` when provided a non-numeric fill value [gh-1878](https://github.com/IntelPython/dpctl/pull/1878) + ### Maintenance * Update black version used in Python code style workflow [gh-1828](https://github.com/IntelPython/dpctl/pull/1828) From 320e5e40fa6b72d240b146ffae15e22d9377bbab Mon Sep 17 00:00:00 2001 From: Nikita Grigorian Date: Wed, 23 Oct 2024 13:37:50 -0700 Subject: [PATCH 4/5] Fix `full` and `full_like` with fill_value that is a NumPy bool NumPy bools do not subclass `numbers.Number` and therefore must be handled separately. Python bools do, so no logic is needed. --- dpctl/tensor/_ctors.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/dpctl/tensor/_ctors.py b/dpctl/tensor/_ctors.py index 824655073c..1e6573a792 100644 --- a/dpctl/tensor/_ctors.py +++ b/dpctl/tensor/_ctors.py @@ -1111,7 +1111,12 @@ def full( sycl_queue=sycl_queue, ) return dpt.copy(dpt.broadcast_to(X, shape), order=order) - elif not isinstance(fill_value, Number): + # TODO: verify if `np.True_` and `np.False_` should be instances of + # Number in NumPy, like other NumPy scalars and like Python bools + # check for `np.bool_` separately as NumPy<2 has no `np.bool` + elif not isinstance(fill_value, Number) and not isinstance( + fill_value, np.bool_ + ): raise TypeError( "`full` array cannot be constructed with value of type " f"{type(fill_value)}" @@ -1486,7 +1491,12 @@ def full_like( ) _manager.add_event_pair(hev, copy_ev) return res - elif not isinstance(fill_value, Number): + # TODO: verify if `np.True_` and `np.False_` should be instances of + # Number in NumPy, like other NumPy scalars and like Python bools + # check for `np.bool_` separately as NumPy<2 has no `np.bool` + elif not isinstance(fill_value, Number) and not isinstance( + fill_value, np.bool_ + ): raise TypeError( "`full` array cannot be constructed with value of type " f"{type(fill_value)}" From 70953582729c033099825ba5e66f25f0d12dee09 Mon Sep 17 00:00:00 2001 From: Nikita Grigorian Date: Wed, 23 Oct 2024 16:01:40 -0700 Subject: [PATCH 5/5] Factor out checks for fill value scalar type into `_validate_fill_value` function --- dpctl/tensor/_ctors.py | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/dpctl/tensor/_ctors.py b/dpctl/tensor/_ctors.py index 1e6573a792..37236cad6b 100644 --- a/dpctl/tensor/_ctors.py +++ b/dpctl/tensor/_ctors.py @@ -1038,6 +1038,19 @@ def _cast_fill_val(fill_val, dt): return fill_val +def _validate_fill_value(fill_val): + """ + Validates that `fill_val` is a numeric or boolean scalar. + """ + # TODO: verify if `np.True_` and `np.False_` should be instances of + # Number in NumPy, like other NumPy scalars and like Python bools + # check for `np.bool_` separately as NumPy<2 has no `np.bool` + if not isinstance(fill_val, Number) and not isinstance(fill_val, np.bool_): + raise TypeError( + f"array cannot be filled with scalar of type {type(fill_val)}" + ) + + def full( shape, fill_value, @@ -1111,16 +1124,8 @@ def full( sycl_queue=sycl_queue, ) return dpt.copy(dpt.broadcast_to(X, shape), order=order) - # TODO: verify if `np.True_` and `np.False_` should be instances of - # Number in NumPy, like other NumPy scalars and like Python bools - # check for `np.bool_` separately as NumPy<2 has no `np.bool` - elif not isinstance(fill_value, Number) and not isinstance( - fill_value, np.bool_ - ): - raise TypeError( - "`full` array cannot be constructed with value of type " - f"{type(fill_value)}" - ) + else: + _validate_fill_value(fill_value) sycl_queue = normalize_queue_device(sycl_queue=sycl_queue, device=device) usm_type = usm_type if usm_type is not None else "device" @@ -1491,16 +1496,8 @@ def full_like( ) _manager.add_event_pair(hev, copy_ev) return res - # TODO: verify if `np.True_` and `np.False_` should be instances of - # Number in NumPy, like other NumPy scalars and like Python bools - # check for `np.bool_` separately as NumPy<2 has no `np.bool` - elif not isinstance(fill_value, Number) and not isinstance( - fill_value, np.bool_ - ): - raise TypeError( - "`full` array cannot be constructed with value of type " - f"{type(fill_value)}" - ) + else: + _validate_fill_value(fill_value) dtype = _get_dtype(dtype, sycl_queue, ref_type=type(fill_value)) res = _empty_like_orderK(x, dtype, usm_type, sycl_queue)