IntelPython / dpctl

Python SYCL bindings and SYCL-based Python Array API library
https://intelpython.github.io/dpctl/
Apache License 2.0
97 stars 29 forks source link

Fixes element-wise comparisons of mixed signed-unsigned integer inputs #1650

Closed ndgrigorian closed 2 months ago

ndgrigorian commented 2 months ago

This pull request addresses issues in comparison functions with mixed integral kinds (signed and unsigned) by explicitly adding overloads for combinations of uint64 and int64 and adding a unique weak-type resolver function for the comparisons.

The resolver function guarantees that negative Python scalars are not used to initialize an array with an unsigned integer data type in the binary comparisons, which makes comparisons between negative Python scalars and unsigned integer arrays work as expected.

Old behavior:

In [1]: import dpctl.tensor as dpt, dpctl, numpy as np

In [2]: x = dpt.arange(10, dtype="u8")

In [3]: -1 < x
Out[3]:
usm_ndarray([ True,  True,  True,  True,  True,  True,  True,  True,
              True,  True])

Now:

In [3]: -1 < x
Out[3]:
usm_ndarray([False, False, False, False, False, False, False, False,
             False, False])

Also slips in a change removing an unnecessary overload to not_equal for mixed float and complex float data types, which are safe for basic casting and not present in other elementwise comparisons.

ndgrigorian commented 2 months ago

@oleksandr-pavlyk This PR does not yet address similar issues in clip, minimum, and maximum.

For minimum and maximum a similar solution for negative Python scalars is possible. clip may be a bit trickier.

github-actions[bot] commented 2 months ago

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. :crossed_fingers:

github-actions[bot] commented 2 months ago

Array API standard conformance tests for dpctl=0.17.0dev0=py310h15de555_303 ran successfully. Passed: 868 Failed: 10 Skipped: 92

github-actions[bot] commented 2 months ago

Array API standard conformance tests for dpctl=0.17.0dev0=py310h15de555_304 ran successfully. Passed: 870 Failed: 8 Skipped: 92

coveralls commented 2 months ago

Coverage Status

coverage: 88.218% (+0.008%) from 88.21% when pulling b559f0a940c607d55042cfbccd1353fc6e51b3ce on mixed-kind-integer-comparisons into 7757857466c2fcfb92e8f8e1ed38e90b35c42327 on master.

ndgrigorian commented 2 months ago

@oleksandr-pavlyk I've added tests and added the _is_weak_dtype utility function.

As of Numpy 2, Numpy will no longer permit converting Python integers to integer arrays where the Python integer is out-of-bounds for the integer array's data type (at least in ufuncs). This is how they resolve the issues with minimum, maximum, and clip with unsigned integer arrays and negative Python integers.

Imitating that behavior would align with the future of Numpy (and therefore should be acceptable to DPNP) and seems like the most sensible path forward since array API disallows promotion for those functions, but will take a little more work. Therefore, I will handle it in a separate PR, and this is ready for review pending CI success.

github-actions[bot] commented 2 months ago

Array API standard conformance tests for dpctl=0.17.0dev0=py310h15de555_312 ran successfully. Passed: 871 Failed: 7 Skipped: 92