Skip to content

Supporting order arg to asnumpy#2980

Open
abagusetty wants to merge 7 commits into
IntelPython:masterfrom
abagusetty:fix-asnumpy-order-2884
Open

Supporting order arg to asnumpy#2980
abagusetty wants to merge 7 commits into
IntelPython:masterfrom
abagusetty:fix-asnumpy-order-2884

Conversation

@abagusetty

Copy link
Copy Markdown
Contributor

Fixed dpnp.asnumpy and dpnp.ndarray.asnumpy ignoring the order arg
Fixes: #2568

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • Have you added documentation for your changes, if necessary?
  • Have you added your changes to the changelog?

abagusetty and others added 2 commits June 26, 2026 17:30
dpnp.asnumpy and dpnp.ndarray.asnumpy accepted an order argument but
dropped it for dpnp_array and usm_ndarray inputs. The underlying
_copy_to_numpy rebuilt the NumPy array from the source strides, so a
non-contiguous source was returned with a non-contiguous layout even
when order='C' was requested, diverging from NumPy/CuPy.

Thread order through the full conversion chain:
- _copy_to_numpy(ary, order='K'): apply layout via np.asarray
- dpt.asnumpy(usm_ary, order='K'): forward order
- dpnp_array.asnumpy(order='C'): forward order
- dpnp.asnumpy(a, order='C'): pass order to both array branches

Public APIs default to 'C' to match NumPy/CuPy; internal helpers default
to 'K' to preserve the existing stride-keeping behavior of their direct
callers (to_numpy, _ctors, _print).

Add TestAsNumpy covering iface/method/usm_ndarray paths and default
semantics.
@intel-python-devops

Copy link
Copy Markdown

Can one of the admins verify this patch?

@antonwolfy antonwolfy added this to the 0.21.0 release milestone Jun 30, 2026
@coveralls

coveralls commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Coverage Status

coverage: 78.071% (-0.002%) from 78.073% — abagusetty:fix-asnumpy-order-2884 into IntelPython:master

Comment thread dpnp/tensor/_copy_utils.py Outdated
Comment thread dpnp/tensor/_copy_utils.py Outdated
Comment thread CHANGELOG.md Outdated
def test_method_order_k_keeps_strides(self):
# explicit "K" keeps the strides of the source as closely as possible
a = self._non_c_contiguous_array()
result = a.asnumpy(order="K")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing test for order="A" and order=None

a = self._non_c_contiguous_array()
result = a.asnumpy(order="K")
assert not result.flags["C_CONTIGUOUS"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Messing test with zero-sized ndarray

@abagusetty

Copy link
Copy Markdown
Contributor Author

@antonwolfy Thanks for taking time. Addressed the above suggestions

@ndgrigorian

ndgrigorian commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

@abagusetty
while this works in some cases, in other edge cases, this will not work, especially in terms of keeping "K" order:

In [1]: import numpy as np

In [2]: import dpnp as dnp

In [3]: x = np.arange(10)

In [4]: x_np = np.arange(10)

In [5]: x = dnp.arange(10)

In [6]: x
Out[6]: array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9])

In [7]: x_np
Out[7]: array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9])

In [8]: x = x[::-1]

In [9]: x_np = x_np[::-1]

In [10]: dnp.asnumpy(x)
Out[10]: array([9, 8, 7, 6, 5, 4, 3, 2, 1, 0])

In [11]: _.strides
Out[11]: (8,)

In [12]: np.asarray(x_np)
Out[12]: array([9, 8, 7, 6, 5, 4, 3, 2, 1, 0])

In [13]: _.strides
Out[13]: (-8,)

the issue is that there are no kernels for copying to strided host memory, only contiguous. So in such a case as this, we get a C-contiguous copy back. With cases like F-contiguous memory, order="K" will preserve it, because we can copy to F-contiguous host memory, as it is contiguous.

A proper solution requires new kernels. At minimum, the limitation should be documented.

Comment on lines +80 to +82
# apply the requested memory layout; ``"K"`` preserves the strides of the
# source array as closely as possible and is the default
return np.asarray(result, order=order)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

part of the reason is that this will preserve the strides of the source result array, which are not necessarily correct per NumPy's order="K" semantics themselves

@ndgrigorian

Copy link
Copy Markdown
Collaborator

@abagusetty
it seems order="K" by default wasn't changed for dnp.asnumpy, so that observation was incorrect!

In [10]: dpt.asnumpy(x_dpt)
Out[10]: array([9, 8, 7, 6, 5, 4, 3, 2, 1, 0])

In [11]: _.strides
Out[11]: (-8,)

In [12]: dnp.asnumpy(x_dnp, order="K")
Out[12]: array([9, 8, 7, 6, 5, 4, 3, 2, 1, 0])

In [13]: _.strides
Out[13]: (-8,)

I will look for other edge cases, but if this works in this case, I don't see a likely issue otherwise

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.

dpnp.asnumpy does not honor order arg

5 participants