-
Notifications
You must be signed in to change notification settings - Fork 22
Use tril() and triu() function from dpctl.tensor #1286
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
Conversation
dpnp/dpnp_iface_arraycreation.py
Outdated
pass | ||
elif x1.ndim < 2: | ||
pass | ||
elif not isinstance(k, int): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be better to implement the same check through operator.index
like dpctl did.
Then the following code will be supported:
a = dpnp.arange(9, dtype=dpnp.int64)
dpnp.tril(a.reshape((3, 3)), k=a[0])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I try and see:
a = dpnp.arange(9, dtype=dpnp.int64)
operator.index(a[0])
TypeError: 'dpnp_array' object cannot be interpreted as an integer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be supported once PR #1285 is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably it'd better something like:
_k = None
try:
_k = operator.index(k)
except TypeError:
pass
if not isinstance(x1, (dpnp.ndarray, dpt.usm_ndarray)):
pass
elif x1.ndim < 2:
pass
elif _k is None:
pass
else:
return dpnp_container.tril(x1, k=_k)
return call_origin(numpy.tril, x1, k)
then we will also cover numpy scalar here.
tests/skipped_tests.tbl
Outdated
@@ -430,6 +430,9 @@ tests/third_party/cupy/creation_tests/test_ranges.py::TestRanges::test_linspace_ | |||
tests/third_party/cupy/creation_tests/test_ranges.py::TestRanges::test_linspace_mixed_start_stop | |||
tests/third_party/cupy/creation_tests/test_ranges.py::TestRanges::test_linspace_mixed_start_stop2 | |||
tests/third_party/cupy/creation_tests/test_ranges.py::TestRanges::test_linspace_start_stop_list | |||
tests/third_party/cupy/creation_tests/test_matrix.py::TestTriLowerAndUpper_param_0_{shape=(2,)}::test_tril |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you mark the tests as to be skipped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dpctl.tensor.tril() does not support one dimension array.
Array API standard definition:
x (array) – input array having shape (..., M, N) and whose innermost two dimensions form MxN matrices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a fixture:
@pytest.mark.usefixtures("allow_fall_back_on_numpy")
before
class TestTriLowerAndUpper(unittest.TestCase):
to allow test to fallback on numpy with shape=(2,) rather then to silently skip it.
Any nested "allow_fall_back_on_numpy" fixtures above class methods can be removed then.
dpnp/dpnp_iface_arraycreation.py
Outdated
pass | ||
elif x1.ndim < 2: | ||
pass | ||
elif not isinstance(k, int): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably it'd better something like:
_k = None
try:
_k = operator.index(k)
except TypeError:
pass
if not isinstance(x1, (dpnp.ndarray, dpt.usm_ndarray)):
pass
elif x1.ndim < 2:
pass
elif _k is None:
pass
else:
return dpnp_container.tril(x1, k=_k)
return call_origin(numpy.tril, x1, k)
then we will also cover numpy scalar here.
tests/test_arraycreation.py
Outdated
@@ -258,48 +258,44 @@ def test_tri_default_dtype(): | |||
|
|||
|
|||
@pytest.mark.parametrize("k", | |||
[-6, -5, -4, -3, -2, -1, 0, 1, 2, 3, 4, 5, 6], | |||
ids=['-6', '-5', '-4', '-3', '-2', '-1', '0', '1', '2', '3', '4', '5', '6']) | |||
[-3, -2, -1, 0, 1, 2, 3, 4, 5], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add here (and in test below) 0-dimension array of dpnp_array and usm_ndarray and numpy scalar?
tests/skipped_tests.tbl
Outdated
@@ -430,6 +430,9 @@ tests/third_party/cupy/creation_tests/test_ranges.py::TestRanges::test_linspace_ | |||
tests/third_party/cupy/creation_tests/test_ranges.py::TestRanges::test_linspace_mixed_start_stop | |||
tests/third_party/cupy/creation_tests/test_ranges.py::TestRanges::test_linspace_mixed_start_stop2 | |||
tests/third_party/cupy/creation_tests/test_ranges.py::TestRanges::test_linspace_start_stop_list | |||
tests/third_party/cupy/creation_tests/test_matrix.py::TestTriLowerAndUpper_param_0_{shape=(2,)}::test_tril |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a fixture:
@pytest.mark.usefixtures("allow_fall_back_on_numpy")
before
class TestTriLowerAndUpper(unittest.TestCase):
to allow test to fallback on numpy with shape=(2,) rather then to silently skip it.
Any nested "allow_fall_back_on_numpy" fixtures above class methods can be removed then.
d38908d
to
6e22482
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
* Complete support of python 3.10 in external CI (#1269) * Set minimum required versions & fix debug building (#1270) * Set minimum required versions & fix debug building * Fix typo * Add support of NumPy 1.24 (#1276) * Set minimum required versions & fix debug building * Add support of numpy 1.24 * Get rid of 'has_aspect_host' property in tests (#1274) * Set minimum required versions & fix debug building * Get rid of 'has_aspect_host' property in tests * Update tests/test_sycl_queue.py Co-authored-by: Oleksandr Pavlyk <[email protected]> Co-authored-by: Oleksandr Pavlyk <[email protected]> * Add support of dpnp.less_equal() (#1275) * Set minimum required versions & fix debug building * Add support of dpnp.less_equal() * Test no broadcast together with input shapes * Add support of comparison operations (#1278) * Use eye() function from dpctl.tensor. (#1271) * Use eye() function from dpctl.tensor. * Add missed order in test for eye() function. * Updated copyright year. Added parameter like for eye() function. * Removed input argumet additional kwards for eye() function. * Get rid of unsupported types in array creation tests (#1283) * Add support of logical comparison operations (#1280) * Add device and sycl_queue keyword arguments to random calls (#1277) * Set minimum required versions & fix debug building * Add device and sycl_queue keyword arguments to random calls * Add device and sycl_queue to dpnp.random.seed() & use random values if seed is None * Update dpnp/random/dpnp_iface_random.py Co-authored-by: Oleksandr Pavlyk <[email protected]> --------- Co-authored-by: Oleksandr Pavlyk <[email protected]> * add __repr__ * add __str__ * reviewer's comments * Fixed gh-1272 (#1287) * Support high=None in dpnp.randint() (#1284) * linter changes applied * Add operation __index__ and __complex__ (#1285) * Add operation __index__ and __complex__ * Add tests * Update tests with proper call of dpctl.SyclQueue() (#1290) * Update minimum required versions of dependent components. (#1289) * dpnp.add() doesn't work properly with a scalar (#1288) * dpnp.add() doesn't work properly with a scalar * get rid of dpctl.SyclQueue() call in tests with unsupported device keyword * Add a fix for crash on CPU device * USM type in operations with a scalar * Porting fix for crash to logic kernel * Add dlpack support with tests and docstrings * Add a test for dlpack with dpt * Fix remarks, add _create_from_usm_ndarray func and move tests to test_sycl_queue * Use tril() and triu() function from dpctl.tensor (#1286) * Use tril() function from dpctl.tensor * Use triu() function from dpctl.tensor * Changed tests for tril() and triu() functions. * Skip tests for tril() and triu() functions with usm_type. * dpnp.subtract() doesn't work properly with a scalar (#1292) * dpnp.add() doesn't work properly with a scalar * dpnp.subtract() doesn't work properly with a scalar * USM type in operations with a scalar * Rollback excluded 'floor_divide' tests from skip scope * Explicit vector operations instead of saturation functions * Use std::int32_t and std::int64_t types * Tune tail's loop of kernel for the vector op * dpnp.divide() doesn't work properly with a scalar (#1295) * dpnp.add() doesn't work properly with a scalar * dpnp.subtract() doesn't work properly with a scalar * dpnp.divide() doesn't work properly with a scalar * dpnp.divide() doesn't work properly with a scalar * Use std::int32_t and std::int64_t types * Disable floating-point optimizations that assume arguments and results are not NaNs or +-Inf * Fix issue with divide on Iris Xe * Updae example3 building from debug build script (#1298) * Remove temporary solution accepting CFD with equal SYCL context instead of queue (#1303) * Intel LLVM is to use conda's gcc toolchain, sysroot and target libraries (#1306) * Tests are crashing if no default device (#1311) * Setting version to 0.11.1 (#1308) --------- Co-authored-by: Oleksandr Pavlyk <[email protected]> Co-authored-by: Natalia Polina <[email protected]> Co-authored-by: Vahid Tavanashad <[email protected]> Co-authored-by: Vladislav Perevezentsev <[email protected]>
Use tril() and triu() functions from dpctl.tensor module instead of DPNP backend implementation.
A cython code relating to tril() and triu() call is cleaned up.