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

`order` of output array for binary elementwise funcs, `clip`, and `matmul` defaults to C-contiguous #1678

Closed ndgrigorian closed 1 month ago

ndgrigorian commented 1 month ago

This PR proposes a change to clip, matmul, and the BinaryElementwiseFunc class which makes C the default contiguity of outputs when order="K" and the input arrays must be cast.

This fixes a minor discrepancy where when multiple input arrays are both C- and F-contiguous and need to be cast, the output defaulted to F-contiguous instead of C-contiguous, creating unusual edge cases where type promotion (and therefore the selected device) became relevant to the output array's order, which should not be the case.

For example

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

In [2]: x1 = dpt.ones((10, 1), dtype="i4", device="cpu")

In [3]: x2 = dpt.ones((1, 5), dtype="f4", device="cpu")

In [4]: r = dpt.multiply(x1, x2)

In [5]: r.flags
Out[5]:
  C_CONTIGUOUS : False
  F_CONTIGUOUS : True
  WRITABLE : True

The selected device's type promotion graph contains float64, leading to F-contiguous outputs.

after this change:

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

In [2]: x1 = dpt.ones((10, 1), dtype="i4", device="cpu")
x
In [3]: x2 = dpt.ones((1, 5), dtype="f4", device="cpu")

In [4]: r = dpt.multiply(x1, x2)

In [5]: r.flags
Out[5]:
  C_CONTIGUOUS : True
  F_CONTIGUOUS : False
  WRITABLE : True
github-actions[bot] commented 1 month ago

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

oleksandr-pavlyk commented 1 month ago

This looks uncontroversial enough for me.

coveralls commented 1 month ago

Coverage Status

coverage: 87.948%. remained the same when pulling eacef2820f3d2718ee873dbcce32f808f876843f on fix-elementwise-order-K-broadcasting into 7bc3124e879029b2defe99b6bfb2166505048aa3 on master.

github-actions[bot] commented 1 month ago

Array API standard conformance tests for dpctl=0.17.0dev0=py310h15de555_352 ran successfully. Passed: 888 Failed: 17 Skipped: 91

ndgrigorian commented 1 month ago

@vtavana confirmed that dpctl test suite was unaffected by this change, I will go ahead and merge it

Internal CI failures also unrelated