-
Notifications
You must be signed in to change notification settings - Fork 188
Support torch validation and add to spmd tests #3003
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
base: main
Are you sure you want to change the base?
Changes from all commits
71641da
f767365
a3f3407
676fead
2f53e2e
4557b84
05d5d0b
aebd13a
d10d40c
f133f7f
33397db
953da06
a9c19f2
6378c0a
b678d9c
0e463a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ | |
| import numpy as np | ||
| import scipy.sparse as sp | ||
|
|
||
| from ..utils._third_party import _is_subclass_fast | ||
| from ..utils._third_party import _is_subclass_fast, is_torch_tensor, lazy_import | ||
|
|
||
|
|
||
| def _supports_buffer_protocol(obj): | ||
|
|
@@ -73,17 +73,35 @@ def _cls_to_sycl_namespace(cls): | |
| raise ValueError(f"SYCL type not recognized: {cls}") | ||
|
|
||
|
|
||
| @lazy_import("array_api_compat") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well written but unfortunately no longer necessary.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how so? |
||
| def _torch_namespace(array_api_compat, array): | ||
| # torch's array API namespace is exposed via array_api_compat, not as a | ||
| # __array_namespace__ attribute on the tensor itself. | ||
| return array_api_compat.get_namespace(array) | ||
|
|
||
|
|
||
| def _is_sycl_array(x): | ||
| # dpnp exposes __sycl_usm_array_interface__; torch xpu tensors do not, so | ||
| # they are detected separately. Both must be recognized regardless of the | ||
| # array_api_dispatch global so compute-follows-data is preserved. | ||
| return hasattr(x, "__sycl_usm_array_interface__") or is_torch_tensor(x) | ||
|
|
||
|
|
||
| def _get_sycl_namespace(*arrays): | ||
| """Get namespace of sycl arrays.""" | ||
|
|
||
| # sycl support designed to work regardless of array_api_dispatch sklearn global value | ||
| sua_iface = {type(x): x for x in arrays if hasattr(x, "__sycl_usm_array_interface__")} | ||
| sua_iface = {type(x): x for x in arrays if _is_sycl_array(x)} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps it'd be easier to first remove support for DPNP without array API and then do these kinds of changes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you believe that would make most sense I could start looking into this
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, would be better to first remove DPNP without array API, since it will allow removing lots of these functions. |
||
|
|
||
| if len(sua_iface) > 1: | ||
| raise ValueError(f"Multiple SYCL types for array inputs: {sua_iface}") | ||
|
|
||
| if sua_iface: | ||
| (X,) = sua_iface.values() | ||
| if is_torch_tensor(X): | ||
| # torch is array-API compliant via array_api_compat; report it as | ||
| # such so downstream conversions keep results as torch tensors. | ||
| return sua_iface, _torch_namespace(X), True | ||
| return ( | ||
| sua_iface, | ||
| _cls_to_sycl_namespace(type(X)), | ||
|
|
||
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.
Maybe this function should return an additional argument 'requires_array_api' or similar, and all the users should take care of activating array API when appropriate.