From 23c8934a0d7bd4df8ac32461feab716b9603191f Mon Sep 17 00:00:00 2001 From: Graham Markall Date: Mon, 3 Oct 2022 11:01:11 +0100 Subject: [PATCH 1/3] Fix #8477: Allow copies with different strides for 0-length data 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 https://github.com/dask/distributed/pull/7089 where a workaround for an observation of this issue was needed. This would not be needed with the fix in this commit. --- numba/cuda/cudadrv/devicearray.py | 4 +++- numba/cuda/tests/cudadrv/test_cuda_ndarray.py | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/numba/cuda/cudadrv/devicearray.py b/numba/cuda/cudadrv/devicearray.py index 651f06bf698..21adaa94cf3 100644 --- a/numba/cuda/cudadrv/devicearray.py +++ b/numba/cuda/cudadrv/devicearray.py @@ -895,6 +895,8 @@ def check_array_compatibility(ary1, ary2): if ary1sq.shape != ary2sq.shape: raise ValueError('incompatible shape: %s vs. %s' % (ary1.shape, ary2.shape)) - if ary1sq.strides != ary2sq.strides: + # We check strides only if the size is nonzero, because strides are + # irrelevant (and can differ) for zero-length copies. + if ary1.size and ary1sq.strides != ary2sq.strides: raise ValueError('incompatible strides: %s vs. %s' % (ary1.strides, ary2.strides)) diff --git a/numba/cuda/tests/cudadrv/test_cuda_ndarray.py b/numba/cuda/tests/cudadrv/test_cuda_ndarray.py index d84d297ebca..af30bed2c7f 100644 --- a/numba/cuda/tests/cudadrv/test_cuda_ndarray.py +++ b/numba/cuda/tests/cudadrv/test_cuda_ndarray.py @@ -420,6 +420,28 @@ def test_bug6697(self): got = np.asarray(dary) self.assertEqual(got.dtype, dary.dtype) + def test_issue_8477(self): + # Ensure that we can copy a zero-length device array to a zero-length + # host array when the strides of the device and host arrays differ - + # this should be possible because the strides are irrelevant when the + # length is zero. For more info see + # https://github.com/numba/numba/issues/8477. + + # Create a device array with shape (0,) and strides (8,) + src = devicearray.DeviceNDArray(shape=(0,), strides=(8,), dtype=np.int8) + + # Create a host array with shape (0,) and strides (0,) + dest = np.ndarray(shape=(0,), strides=(0,), dtype=np.int8) + + # Sanity check for this test - ensure our destination has the strides + # we expect, because strides can be ignored in some cases by the + # ndarray constructor - checking here ensures that we haven't failed to + # account for unexpected behaviour across different versions of NumPy + self.assertEqual(dest.strides, (0,)) + + # Ensure that the copy succeeds + src.copy_to_host(dest) + class TestRecarray(CUDATestCase): def test_recarray(self): From a3716cd787dc47d918d5fadddd513ffade57bcec Mon Sep 17 00:00:00 2001 From: Graham Markall Date: Mon, 3 Oct 2022 11:27:08 +0100 Subject: [PATCH 2/3] CUDA: Test copies in all directions for #8477 fix --- numba/cuda/tests/cudadrv/test_cuda_ndarray.py | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/numba/cuda/tests/cudadrv/test_cuda_ndarray.py b/numba/cuda/tests/cudadrv/test_cuda_ndarray.py index af30bed2c7f..ee84b3097fd 100644 --- a/numba/cuda/tests/cudadrv/test_cuda_ndarray.py +++ b/numba/cuda/tests/cudadrv/test_cuda_ndarray.py @@ -428,19 +428,31 @@ def test_issue_8477(self): # https://github.com/numba/numba/issues/8477. # Create a device array with shape (0,) and strides (8,) - src = devicearray.DeviceNDArray(shape=(0,), strides=(8,), dtype=np.int8) + dev_array = devicearray.DeviceNDArray(shape=(0,), strides=(8,), + dtype=np.int8) # Create a host array with shape (0,) and strides (0,) - dest = np.ndarray(shape=(0,), strides=(0,), dtype=np.int8) + host_array = np.ndarray(shape=(0,), strides=(0,), dtype=np.int8) # Sanity check for this test - ensure our destination has the strides # we expect, because strides can be ignored in some cases by the # ndarray constructor - checking here ensures that we haven't failed to # account for unexpected behaviour across different versions of NumPy - self.assertEqual(dest.strides, (0,)) + self.assertEqual(host_array.strides, (0,)) - # Ensure that the copy succeeds - src.copy_to_host(dest) + # Ensure that the copy succeeds in both directions + dev_array.copy_to_host(host_array) + dev_array.copy_to_device(host_array) + + # Ensure that a device-to-device copy also succeeds when the strides + # differ - one way of doing this is to copy the host array across and + # use that for copies in both directions. + dev_array_from_host = cuda.to_device(host_array) + self.assertEqual(dev_array_from_host.shape, (0,)) + self.assertEqual(dev_array_from_host.strides, (0,)) + + dev_array.copy_to_device(dev_array_from_host) + dev_array_from_host.copy_to_device(dev_array) class TestRecarray(CUDATestCase): From 9546e5804e77d876bf1655eda0806621c05a20b0 Mon Sep 17 00:00:00 2001 From: Graham Markall Date: Mon, 3 Oct 2022 11:40:41 +0100 Subject: [PATCH 3/3] CUDA: Skip test for #8477 fix in cudasim --- numba/cuda/tests/cudadrv/test_cuda_ndarray.py | 1 + 1 file changed, 1 insertion(+) diff --git a/numba/cuda/tests/cudadrv/test_cuda_ndarray.py b/numba/cuda/tests/cudadrv/test_cuda_ndarray.py index ee84b3097fd..1e674df7aca 100644 --- a/numba/cuda/tests/cudadrv/test_cuda_ndarray.py +++ b/numba/cuda/tests/cudadrv/test_cuda_ndarray.py @@ -420,6 +420,7 @@ def test_bug6697(self): got = np.asarray(dary) self.assertEqual(got.dtype, dary.dtype) + @skip_on_cudasim('DeviceNDArray class not present in simulator') def test_issue_8477(self): # Ensure that we can copy a zero-length device array to a zero-length # host array when the strides of the device and host arrays differ -