Skip to content
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

Feature/asarray #646

Merged
merged 5 commits into from
Nov 23, 2021
Merged

Feature/asarray #646

merged 5 commits into from
Nov 23, 2021

Conversation

oleksandr-pavlyk
Copy link
Collaborator

Closes #640 . Adds dpctl.tensor.asarray and dpctl.tensor.empty.

dpctl.tensor.empty(shape, dtype="f8", order="C", device=None, usm_type="device", sycl_queue=None)

Create an empty dpctl.tensor.usm_ndarray object of requested dtype and based on request USM allocation type on specified array-API device, and of requested layout specified by order keyword.

Instead of specifying array-API device keyword, sycl_queue can be used to specify the target of allocation instead.

dpctl.tensor.asarray(obj, dtype=None, copy=None, order="K", device=None, usm_type=None, sycl_queue=None)

Converts Python object obj to usm_ndarray object. Support objects are:

  1. usm_ndarray instances
  2. numpy.ndarray instances
  3. Python objects exposing __sycl_usm_array_interface__.
  4. Python sequences of Python scalars

@coveralls
Copy link
Collaborator

coveralls commented Oct 29, 2021

Coverage Status

Coverage increased (+0.4%) to 74.788% when pulling 12d6693 on feature/asarray into 749be95 on master.

@oleksandr-pavlyk oleksandr-pavlyk marked this pull request as ready for review November 3, 2021 18:41
@oleksandr-pavlyk oleksandr-pavlyk marked this pull request as draft November 3, 2021 19:14
@reazulhoque
Copy link
Contributor

@oleksandr-pavlyk Do we have a test where we are testing the equality of shape of new array created using asarray and the shape is multi-dimensional? Can you please point me to it?

@oleksandr-pavlyk
Copy link
Collaborator Author

Is this good enough?

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

In [2]: dpt.asarray([[1, 2, 3], [4,5,6]], dtype="d").shape
Out[2]: (2, 3)

In [3]: X = dpt.asarray([[1, 2, 3], [4,5,6]], dtype="d", device="cpu", usm_type="device")

In [4]: Y = dpt.asarray(X, device="gpu", usm_type="shared")

In [5]: X.shape == Y.shape
Out[5]: True

In [6]: X.sycl_device
Out[6]: <dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7f8836ab93b0>

In [7]: Y.sycl_device
Out[7]: <dpctl.SyclDevice [backend_type.level_zero, device_type.gpu,  Intel(R) UHD Graphics [0x9bca]] at 0x7f8858a49870>

@PokhodenkoSA
Copy link
Contributor

PokhodenkoSA commented Nov 9, 2021

The description of this PR has more documentation than functions docstrings you created in code.

"""Represents object `obj` as usm_ndarray"""
# 1. Check that copy is a valid keyword
if copy not in [None, True, False]:
raise TypeError(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ValueError is more suitable here.

Copy link
Collaborator Author

@oleksandr-pavlyk oleksandr-pavlyk Nov 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree, in my mind the check is asking if copy is of bool type, or of None type.

@github-actions
Copy link

dpctl/tensor/_ctors.py Outdated Show resolved Hide resolved
dpctl/tensor/_ctors.py Show resolved Hide resolved
dpctl/tensor/_ctors.py Show resolved Hide resolved
@oleksandr-pavlyk oleksandr-pavlyk force-pushed the feature/asarray branch 2 times, most recently from 9f0c8c2 to 7a580f6 Compare November 19, 2021 19:51
@oleksandr-pavlyk oleksandr-pavlyk marked this pull request as ready for review November 19, 2021 19:54
@oleksandr-pavlyk
Copy link
Collaborator Author

@diptorupd @densmirn @reazulhoque @Alexander-Makaryev @PokhodenkoSA

This PR is now feature complete and changed from draft to "ready for review".
I added docstrings, added CHANGELOG, added tests (guides by coverage).

I cleaned up commit history to reduce it from 32 commits to 10 commit,
I do not consider it a good use of our time to split this PR into any number of smaller PRs.

```
dpctl.tensor.asarray(
   obj, dtype=None, copy=None, order="C",
   device=None, usm_type=None, sycl_queue=None
)
```

```
dpctl.tensor.empty(
    shape,
    dtype="f8", order="C", device=None,
    usm_type=None, sycl_queue=None
)
```
1. Check validation of arguments is `asarray`
2. Check that `asarray` works with objects exposing `__sycl_usm_array_interface__`.

Adding a test with scalar arguments, tests to cover input validation,
tests for copy=False
Change default of asarray's order keyword from "K" to "C"
Fixed issue pointed out by Denis (processing of order in empty)
Added support for objects with buffer protocol in asarray.
@oleksandr-pavlyk
Copy link
Collaborator Author

The description of this PR has more documentation than functions docstrings you created in code.

I added docstrings.

Copy link
Contributor

@reazulhoque reazulhoque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested manually with DuckUSMArray to make sure integration with Numba-dppy is working correctly.

@oleksandr-pavlyk oleksandr-pavlyk merged commit ed8d6ef into master Nov 23, 2021
@oleksandr-pavlyk oleksandr-pavlyk deleted the feature/asarray branch November 23, 2021 20:41
@github-actions
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support dpctl.asarray()
6 participants