privatize pk.array - #406
Conversation
| def array( | ||
| array, space: Optional[MemorySpace] = None, layout: Optional[Layout] = None | ||
| ) -> ViewType: | ||
| """ | ||
| Deprecated public compatibility shim for internal array conversion. | ||
|
|
||
| Prefer `pk.asarray` for user-level conversions. | ||
| """ | ||
|
|
||
| warnings.warn( | ||
| "pk.array is deprecated and will be removed in a future release. " | ||
| "Use pk.asarray for user conversions; pk._array is internal/private.", | ||
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
|
|
||
| return _array(array, space, layout) | ||
|
|
There was a problem hiding this comment.
We can remove it completely.
No need to give warnings.
| is_array_api_constant = False | ||
| if np.isscalar(obj): | ||
| if obj in (pk.e, pk.pi, pk.inf): | ||
| is_array_api_constant = True | ||
| else: | ||
| try: | ||
| is_array_api_constant = bool(np.isnan(obj)) | ||
| except TypeError: | ||
| is_array_api_constant = False | ||
|
|
||
| if is_array_api_constant: |
There was a problem hiding this comment.
What's the point of it? Why we can't use pk.nan?
And it looks invalid since we can test cupy arrays with np.isnan, which should give errors.
| self.np_view: pk.View2D[int] = pk.asarray(np_arr) | ||
| self.cp_view: pk.View2D[int] = pk._array(cp_arr) | ||
| self.list_view: pk.View2D[int] = pk.asarray(list_arr) |
There was a problem hiding this comment.
The end goal is not to use pykokkos internal data structures. If we will deprecate array but still will use asarray - that's not correct.
We want to have something like self.np_view = np.array(...) and be able to use it like that.
To be 100% flexible, we can have some intermediate data structure (per-file only) like xp that should decide if we want to use cupy or numpy based on current execution space.
But there should not be xp.asarray(numpy_array). It should be xp.array(...).
You can treat xp as a C++ macros like this:
#ifdef __CUDA_ARCH__
xp = cupy
#else
xp = numpy
#endif| assert_allclose(from_private, from_public) | ||
|
|
||
|
|
||
| @pytest.mark.skipif(not HAS_CUDA, reason="CUDA/cupy not available") |
There was a problem hiding this comment.
This one is cool. We should add more of this + more CUDA test.
2 participants
Closes #388.
Renames
pk.arraytopk._arrayas the internal conversion function.pk.arrayis kept temporarily as a compatibility wrapper that emits aDeprecationWarningand forwards topk._array. User-facing call sites are migrated topk.asarray, while CuPy/GPU paths usepk._arraydirectly to preserve conversion semantics. Deprecation and compatibility tests are added covering NumPy arrays, lists, scalars, and CuPy (CUDA-gated).