test_serialize_numba: Workaround issue with np.empty_like in NumPy 1.23 #7089
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In NumPy 1.23, the strides of empty arrays are 0 instead of the item size, due to numpy/numpy#21477 - however, `np.empty_like` seems to create non-zero-strided arrays from a zero-strided empty array, and copying to the host from a device array with zero strides fails a compatibility check in Numba. This commit works around the issue by calling `copy_to_host()` with no arguments, allowing Numba to create an array on the host that is compatible with the device array - the resulting implementation is functionally equivalent and slightly simpler, so I believe this change could remain permanant rather than requiring a revert later.
|
Can one of the admins verify this patch? Admins can comment |
2 tasks
|
add to allowlist |
|
ok to test |
|
Just to confirm, we expect this to also work with NumPy 1.22.X as well, correct ? |
Yes. |
5 tasks
|
Presumably the fails here are unrelated to this PR? |
Closed
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 15 files ±0 15 suites ±0 6h 29m 8s ⏱️ - 5m 20s Results for commit 84d776a. ± Comparison against base commit ee43309. ♻️ This comment has been updated with latest results. |
|
I think this is ok to merge in but I will wait for until morning to do so. In the mean time I filed to #7092 to track the unrelated test failure we see here: |
|
All tests are passing now. Merging in. |
|
Related Numba issue: numba/numba#8477 |
gmarkall
added a commit
to gmarkall/numba
that referenced
this pull request
Oct 3, 2022
Since NumPy 1.23, it is possible for zero-length ndarrays to have different strides (e.g. `(0,)`, and `(8,)`). If we attempt to copy from a zero-length device array to a zero-length host array where the strides differ, our compatibility check fails because it compares strides. This commit fixes the issue by only considering strides when checking compatibility of nonzero-length arrays. I believe this to be valid because the following works normally with NumPy 1.23: ```python import numpy as np ary1 = np.arange(0) ary2 = np.ndarray((0,), buffer=ary1.data) ary3 = np.empty_like(ary2) ary3[:] = ary2 ary3[...] = ary2 np.copyto(ary2, ary3) ``` i.e. copying zero-length arrays with different strides generally works as expected. The included test is written in such a way that it should test this change in behaviour regardless of the installed NumPy version - we explicitly construct zero-length device and host arrays with differing strides. The additional sanity check ensures that the host array has the strides we expect, just in case there is some version of NumPy in which setting the strides explicitly didn't result in the expected strides - I have observed that requesting nonzero strides for a zero length array can result still in zero strides (a separate but related behaviour), so this sanity check is provided to account for any other unexpected behaviour of this nature. I have tested locally with NumPy 1.22 and 1.23 (pre- and post-changes to strides). See also dask/distributed#7089 where a workaround for an observation of this issue was needed. This would not be needed with the fix in this commit.
gjoseph92
pushed a commit
to gjoseph92/distributed
that referenced
this pull request
Oct 31, 2022
…ask#7089) In NumPy 1.23, the strides of empty arrays are 0 instead of the item size, due to numpy/numpy#21477 - however, `np.empty_like` seems to create non-zero-strided arrays from a zero-strided empty array, and copying to the host from a device array with zero strides fails a compatibility check in Numba. This commit works around the issue by calling `copy_to_host()` with no arguments, allowing Numba to create an array on the host that is compatible with the device array - the resulting implementation is functionally equivalent and slightly simpler, so I believe this change could remain permanant rather than requiring a revert later.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In NumPy 1.23, the strides of empty arrays are 0 instead of the item size, due to numpy/numpy#21477 - however,
np.empty_likeseems to create non-zero-strided arrays from a zero-strided empty array, and copying to the host from a device array with zero strides fails a compatibility check in Numba.This PR works around the issue by calling
copy_to_host()with no arguments, allowing Numba to create an array on the host that is compatible with the device array - the resulting implementation is functionally equivalent and slightly simpler, so I believe this change could remain permanant rather than requiring a revert later.pre-commit run --all-files