From e61310d543a29ed765d43cec741c145cb3df117d Mon Sep 17 00:00:00 2001 From: Irina Demeshko Date: Mon, 1 Aug 2022 09:48:44 -0600 Subject: [PATCH 1/8] adding support for array shape () passed as an index argument in advanced indexing --- cunumeric/deferred.py | 7 ++++++- src/cunumeric/index/zip_template.inl | 10 +++++++--- tests/integration/test_set_item.py | 4 ++-- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/cunumeric/deferred.py b/cunumeric/deferred.py index a5117405c..f58ff8bdc 100644 --- a/cunumeric/deferred.py +++ b/cunumeric/deferred.py @@ -446,6 +446,10 @@ def _zip_indices( # NumPy array. N = self.ndim pointN_dtype = self.runtime.get_point_type(N) + # if scalar array is passed as an argument, make the output + # shape be (1,) + if out_shape == (): + out_shape = (1,) store = self.context.create_store( pointN_dtype, shape=out_shape, optimize_scalar=True ) @@ -463,7 +467,8 @@ def _zip_indices( task.add_scalar_arg(self.shape, (ty.int64,)) for a in arrays: task.add_input(a) - task.add_alignment(output_arr.base, a) + if a.shape != (): + task.add_alignment(output_arr.base, a) task.execute() return output_arr diff --git a/src/cunumeric/index/zip_template.inl b/src/cunumeric/index/zip_template.inl index cd772ae43..66e6af2cb 100644 --- a/src/cunumeric/index/zip_template.inl +++ b/src/cunumeric/index/zip_template.inl @@ -67,12 +67,12 @@ struct ZipImpl { template static void zip_template(TaskContext& context) { - // Here `N` is the number of dimenstions of the input array and the number + // Here `N` is the number of dimensions of the input array and the number // of dimensions of the Point field // key_dim - is the number of dimensions of the index arrays before // they were broadcasted to the shape of the input array (shape of // all index arrays should be the same)) - // start index - is the index from wich first index array was passed + // start index - is the index from which first index array was passed // DIM - dimension of the output array // // for the example: @@ -95,7 +95,11 @@ static void zip_template(TaskContext& context) int64_t start_index = context.scalars()[2].value(); auto shape = context.scalars()[3].value(); ZipArgs args{context.outputs()[0], context.inputs(), N, key_dim, start_index, shape}; - double_dispatch(args.inputs[0].dim(), N, ZipImpl{}, args); + int dim = args.inputs[0].dim(); + // if scalar passed as an input, convert it to the array size 1 + if (dim == 0) { dim = 1; } + + double_dispatch(dim, N, ZipImpl{}, args); } } // namespace cunumeric diff --git a/tests/integration/test_set_item.py b/tests/integration/test_set_item.py index 55c46b5a5..2314916f5 100644 --- a/tests/integration/test_set_item.py +++ b/tests/integration/test_set_item.py @@ -40,9 +40,9 @@ def test_basic(): def test_scalar_ndarray_as_index(arr): offsets = num.arange(5) # [0, 1, 2, 3, 4] offset = offsets[3] # 3 - # arr[offset] = -1 # TODO: doesn't work when arr is a num.ndarray + arr[offset] = -1 arr[offset - 2 : offset] = [-1, -1] - assert np.array_equal(arr, [4, -1, -1, 1, 0]) + assert np.array_equal(arr, [4, -1, -1, -1, 0]) if __name__ == "__main__": From 841ff41acabe8d461247b1040204b3de76d77338 Mon Sep 17 00:00:00 2001 From: Irina Demeshko Date: Tue, 2 Aug 2022 13:24:25 -0600 Subject: [PATCH 2/8] fxing logic for some advanced_indexing test cases --- cunumeric/deferred.py | 10 +++--- tests/integration/test_advanced_indexing.py | 35 +++++++++++++++++++++ tests/integration/test_get_item.py | 2 +- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/cunumeric/deferred.py b/cunumeric/deferred.py index f58ff8bdc..088a825cd 100644 --- a/cunumeric/deferred.py +++ b/cunumeric/deferred.py @@ -446,10 +446,6 @@ def _zip_indices( # NumPy array. N = self.ndim pointN_dtype = self.runtime.get_point_type(N) - # if scalar array is passed as an argument, make the output - # shape be (1,) - if out_shape == (): - out_shape = (1,) store = self.context.create_store( pointN_dtype, shape=out_shape, optimize_scalar=True ) @@ -742,7 +738,11 @@ def get_item(self, key: Any) -> NumPyThunk: index_array, self, ) = self._create_indexing_array(key) + + if rhs.base.kind == Future: + rhs = self._convert_future_to_store(rhs) store = rhs.base + if copy_needed: result: NumPyThunk if index_array.base.kind == Future: @@ -827,6 +827,8 @@ def set_item(self, key: Any, rhs: Any) -> None: if index_array.base.kind == Future: index_array = self._convert_future_to_store(index_array) + if lhs.base.kind == Future: + lhs = self._convert_future_to_store(lhs) copy = self.context.create_copy() copy.set_target_indirect_out_of_range(False) diff --git a/tests/integration/test_advanced_indexing.py b/tests/integration/test_advanced_indexing.py index e34f6dea5..f69466665 100644 --- a/tests/integration/test_advanced_indexing.py +++ b/tests/integration/test_advanced_indexing.py @@ -23,6 +23,41 @@ from legate.core import LEGATE_MAX_DIM +def test_future_stores(): + # array is a future: + arr_np = np.array([4]) + index_np = np.zeros(8, dtype=int) + arr_num = num.array(arr_np) + index_num = num.array(index_np) + res_np = arr_np[index_np] + res_num = arr_num[index_num] + assert np.array_equal(res_np, res_num) + + # index and array and lhs are futures: + res_np = arr_np[index_np[1]] + res_num = arr_num[index_num[1]] + assert np.array_equal(res_np, res_num) + + # all futures + b_np = np.array([10, 11, 12]) + b_num = num.array(b_np) + arr_np[index_np[1]] = b_np[0] + arr_num[index_num[1]] = b_num[0] + assert np.array_equal(arr_np, arr_num) + + # index and lhs are futures: + arr_np = np.array([4, 3, 2, 1]) + arr_num = num.array(arr_np) + res_np = arr_np[index_np[3]] + res_num = arr_num[index_num[3]] + assert np.array_equal(res_np, res_num) + + # rhs is a future + arr_np[index_np[3]] = b_np[2] + arr_num[index_num[3]] = b_num[2] + assert np.array_equal(arr_np, arr_num) + + def test(): # tests on 1D input array: diff --git a/tests/integration/test_get_item.py b/tests/integration/test_get_item.py index be7f9694d..9f540caf9 100644 --- a/tests/integration/test_get_item.py +++ b/tests/integration/test_get_item.py @@ -36,7 +36,7 @@ def test_basic(): def test_scalar_ndarray_as_index(arr): offsets = num.arange(5) # [0, 1, 2, 3, 4] offset = offsets[3] # 3 - # assert arr[offset] == 1 # TODO: doesn't work when arr is a num.ndarray + assert arr[offset] == 1 assert np.array_equal(arr[offset - 2 : offset], [3, 2]) From ebfa420cf2d3327eb80a0478028f171434cbbfe6 Mon Sep 17 00:00:00 2001 From: Irina Demeshko Date: Tue, 2 Aug 2022 19:41:47 -0600 Subject: [PATCH 3/8] improving the test --- tests/integration/test_get_item.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_get_item.py b/tests/integration/test_get_item.py index 9f540caf9..14af5f004 100644 --- a/tests/integration/test_get_item.py +++ b/tests/integration/test_get_item.py @@ -36,7 +36,7 @@ def test_basic(): def test_scalar_ndarray_as_index(arr): offsets = num.arange(5) # [0, 1, 2, 3, 4] offset = offsets[3] # 3 - assert arr[offset] == 1 + assert np.array_equal(arr[offset], 1) assert np.array_equal(arr[offset - 2 : offset], [3, 2]) From 620109a714c86381d8702cc3cc3fedd46926f813 Mon Sep 17 00:00:00 2001 From: Irina Demeshko Date: Wed, 3 Aug 2022 12:12:40 -0600 Subject: [PATCH 4/8] addressing PR comments --- cunumeric/deferred.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cunumeric/deferred.py b/cunumeric/deferred.py index 088a825cd..c99c1c9e8 100644 --- a/cunumeric/deferred.py +++ b/cunumeric/deferred.py @@ -463,8 +463,7 @@ def _zip_indices( task.add_scalar_arg(self.shape, (ty.int64,)) for a in arrays: task.add_input(a) - if a.shape != (): - task.add_alignment(output_arr.base, a) + task.add_alignment(output_arr.base, a) task.execute() return output_arr @@ -739,11 +738,12 @@ def get_item(self, key: Any) -> NumPyThunk: self, ) = self._create_indexing_array(key) - if rhs.base.kind == Future: - rhs = self._convert_future_to_store(rhs) store = rhs.base if copy_needed: + if rhs.base.kind == Future: + rhs = self._convert_future_to_store(rhs) + store = rhs.base result: NumPyThunk if index_array.base.kind == Future: index_array = self._convert_future_to_store(index_array) From 8bbb99074d3bacbfacd9fe404e90f80575ae0906 Mon Sep 17 00:00:00 2001 From: Manolis Papadakis Date: Wed, 3 Aug 2022 11:29:45 -0700 Subject: [PATCH 5/8] Add some further scalar advanced indexing tests --- tests/integration/test_advanced_indexing.py | 78 +++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/tests/integration/test_advanced_indexing.py b/tests/integration/test_advanced_indexing.py index f69466665..d75e277e3 100644 --- a/tests/integration/test_advanced_indexing.py +++ b/tests/integration/test_advanced_indexing.py @@ -23,6 +23,84 @@ from legate.core import LEGATE_MAX_DIM +# arr = [42] +# idx = 0 +@pytest.mark.parametrize("arr_is_region", [True, False]) +@pytest.mark.parametrize("idx_is_region", [True, False]) # Fails +def test_getitem_scalar_0d(arr_is_region, idx_is_region): + if arr_is_region: + arr = num.full((5,), 42)[2:3] + else: + arr = num.full((1,), 42) + if idx_is_region: + idx = num.zeros((3,), dtype=np.int64)[2:3].reshape(()) + else: + idx = num.zeros((3,), dtype=np.int64).max() + assert np.array_equal(arr[idx], 42) + + +# arr = [42] +# idx = 0 +# val = -1 +@pytest.mark.parametrize("arr_is_region", [True, False]) +@pytest.mark.parametrize("idx_is_region", [True, False]) # Fails +@pytest.mark.parametrize("val_is_region", [True, False]) # Fails +def test_setitem_scalar_0d(arr_is_region, idx_is_region, val_is_region): + if arr_is_region: + arr = num.full((5,), 42)[2:3] + else: + arr = num.full((1,), 42) + if idx_is_region: + idx = num.zeros((3,), dtype=np.int64)[2:3].reshape(()) + else: + idx = num.zeros((3,), dtype=np.int64).max() + if val_is_region: + val = num.full((3,), -1)[2:3].reshape(()) + else: + val = num.full((3,), -1).max() + arr[idx] = val + assert np.array_equal(arr, [-1]) + + +# arr = [42] +# idx = [0] +@pytest.mark.parametrize("arr_is_region", [True, False]) +@pytest.mark.parametrize("idx_is_region", [True, False]) +def test_getitem_scalar_1d(arr_is_region, idx_is_region): + if arr_is_region: + arr = num.full((5,), 42)[2:3] + else: + arr = num.full((1,), 42) + if idx_is_region: + idx = num.zeros((3,), dtype=np.int64)[2:3] + else: + idx = num.zeros((1,), dtype=np.int64) + assert np.array_equal(arr[idx], [42]) + + +# arr = [42] +# idx = [0] +# val = [-1] +@pytest.mark.parametrize("arr_is_region", [True, False]) +@pytest.mark.parametrize("idx_is_region", [True, False]) +@pytest.mark.parametrize("val_is_region", [True, False]) +def test_setitem_scalar_1d(arr_is_region, idx_is_region, val_is_region): + if arr_is_region: + arr = num.full((5,), 42)[2:3] + else: + arr = num.full((1,), 42) + if idx_is_region: + idx = num.zeros((3,), dtype=np.int64)[2:3] + else: + idx = num.zeros((1,), dtype=np.int64) + if val_is_region: + val = num.full((3,), -1)[2:3] + else: + val = num.full((1,), -1) + arr[idx] = val + assert np.array_equal(arr, [-1]) + + def test_future_stores(): # array is a future: arr_np = np.array([4]) From 69d4eca5d6dcf438e466b2904c3563acc8fa27c0 Mon Sep 17 00:00:00 2001 From: Manolis Papadakis Date: Wed, 3 Aug 2022 12:57:57 -0700 Subject: [PATCH 6/8] Skip some problematic tests for now --- tests/integration/test_advanced_indexing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_advanced_indexing.py b/tests/integration/test_advanced_indexing.py index d75e277e3..45f1fa12d 100644 --- a/tests/integration/test_advanced_indexing.py +++ b/tests/integration/test_advanced_indexing.py @@ -26,7 +26,7 @@ # arr = [42] # idx = 0 @pytest.mark.parametrize("arr_is_region", [True, False]) -@pytest.mark.parametrize("idx_is_region", [True, False]) # Fails +@pytest.mark.parametrize("idx_is_region", [False]) # TODO: 'True' Fails def test_getitem_scalar_0d(arr_is_region, idx_is_region): if arr_is_region: arr = num.full((5,), 42)[2:3] @@ -43,8 +43,8 @@ def test_getitem_scalar_0d(arr_is_region, idx_is_region): # idx = 0 # val = -1 @pytest.mark.parametrize("arr_is_region", [True, False]) -@pytest.mark.parametrize("idx_is_region", [True, False]) # Fails -@pytest.mark.parametrize("val_is_region", [True, False]) # Fails +@pytest.mark.parametrize("idx_is_region", [False]) # TODO: 'True' Fails +@pytest.mark.parametrize("val_is_region", [False]) # TODO: 'True' Fails def test_setitem_scalar_0d(arr_is_region, idx_is_region, val_is_region): if arr_is_region: arr = num.full((5,), 42)[2:3] From 51d1dcb166698026c64b6b4f937feb2965615488 Mon Sep 17 00:00:00 2001 From: Manolis Papadakis Date: Wed, 3 Aug 2022 14:37:53 -0700 Subject: [PATCH 7/8] Reformatting of new testcases by @bryevdv --- tests/integration/test_advanced_indexing.py | 108 ++++++++------------ 1 file changed, 44 insertions(+), 64 deletions(-) diff --git a/tests/integration/test_advanced_indexing.py b/tests/integration/test_advanced_indexing.py index 45f1fa12d..d545107d9 100644 --- a/tests/integration/test_advanced_indexing.py +++ b/tests/integration/test_advanced_indexing.py @@ -17,86 +17,66 @@ import numpy as np import pytest +from pytest_lazyfixture import lazy_fixture from utils.generators import mk_seq_array import cunumeric as num from legate.core import LEGATE_MAX_DIM -# arr = [42] -# idx = 0 -@pytest.mark.parametrize("arr_is_region", [True, False]) -@pytest.mark.parametrize("idx_is_region", [False]) # TODO: 'True' Fails -def test_getitem_scalar_0d(arr_is_region, idx_is_region): - if arr_is_region: - arr = num.full((5,), 42)[2:3] - else: - arr = num.full((1,), 42) - if idx_is_region: - idx = num.zeros((3,), dtype=np.int64)[2:3].reshape(()) - else: - idx = num.zeros((3,), dtype=np.int64).max() +@pytest.fixture +def arr_region(): + return num.full((5,), 42)[2:3] + + +@pytest.fixture +def arr_future(): + return num.full((1,), 42) + + +idx_region_1d = num.zeros((3,), dtype=np.int64)[2:3] +idx_future_1d = num.zeros((1,), dtype=np.int64) +idx_region_0d = num.zeros((3,), dtype=np.int64)[2:3].reshape(()) +idx_future_0d = num.zeros((3,), dtype=np.int64).max() + +val_region_1d = num.full((3,), -1)[2:3] +val_future_1d = num.full((1,), -1) +val_region_0d = num.full((3,), -1)[2:3].reshape(()) +val_future_0d = num.full((3,), -1).max() + +# We use fixtures for `arr` because the `set_item` tests modify +# their input. +ARRS = (lazy_fixture("arr_region"), lazy_fixture("arr_future")) +IDXS_0D = (idx_future_0d,) # TODO: idx_region_0d fails +VALS_0D = (val_future_0d,) # TODO: val_region_0d fails +IDXS_1D = (idx_region_1d, idx_future_1d) +VALS_1D = (val_region_1d, val_future_1d) + + +@pytest.mark.parametrize("idx", IDXS_0D) # idx = 0 +@pytest.mark.parametrize("arr", ARRS) # arr = [42] +def test_getitem_scalar_0d(arr, idx): assert np.array_equal(arr[idx], 42) -# arr = [42] -# idx = 0 -# val = -1 -@pytest.mark.parametrize("arr_is_region", [True, False]) -@pytest.mark.parametrize("idx_is_region", [False]) # TODO: 'True' Fails -@pytest.mark.parametrize("val_is_region", [False]) # TODO: 'True' Fails -def test_setitem_scalar_0d(arr_is_region, idx_is_region, val_is_region): - if arr_is_region: - arr = num.full((5,), 42)[2:3] - else: - arr = num.full((1,), 42) - if idx_is_region: - idx = num.zeros((3,), dtype=np.int64)[2:3].reshape(()) - else: - idx = num.zeros((3,), dtype=np.int64).max() - if val_is_region: - val = num.full((3,), -1)[2:3].reshape(()) - else: - val = num.full((3,), -1).max() +@pytest.mark.parametrize("val", VALS_0D) # val = -1 +@pytest.mark.parametrize("idx", IDXS_0D) # idx = 0 +@pytest.mark.parametrize("arr", ARRS) # arr = [42] +def test_setitem_scalar_0d(arr, idx, val): arr[idx] = val assert np.array_equal(arr, [-1]) -# arr = [42] -# idx = [0] -@pytest.mark.parametrize("arr_is_region", [True, False]) -@pytest.mark.parametrize("idx_is_region", [True, False]) -def test_getitem_scalar_1d(arr_is_region, idx_is_region): - if arr_is_region: - arr = num.full((5,), 42)[2:3] - else: - arr = num.full((1,), 42) - if idx_is_region: - idx = num.zeros((3,), dtype=np.int64)[2:3] - else: - idx = num.zeros((1,), dtype=np.int64) +@pytest.mark.parametrize("idx", IDXS_1D) # idx = [0] +@pytest.mark.parametrize("arr", ARRS) # arr = [42] +def test_getitem_scalar_1d(arr, idx): assert np.array_equal(arr[idx], [42]) -# arr = [42] -# idx = [0] -# val = [-1] -@pytest.mark.parametrize("arr_is_region", [True, False]) -@pytest.mark.parametrize("idx_is_region", [True, False]) -@pytest.mark.parametrize("val_is_region", [True, False]) -def test_setitem_scalar_1d(arr_is_region, idx_is_region, val_is_region): - if arr_is_region: - arr = num.full((5,), 42)[2:3] - else: - arr = num.full((1,), 42) - if idx_is_region: - idx = num.zeros((3,), dtype=np.int64)[2:3] - else: - idx = num.zeros((1,), dtype=np.int64) - if val_is_region: - val = num.full((3,), -1)[2:3] - else: - val = num.full((1,), -1) +@pytest.mark.parametrize("val", VALS_1D) # val = [-1] +@pytest.mark.parametrize("idx", IDXS_1D) # idx = [0] +@pytest.mark.parametrize("arr", ARRS) # arr = [42] +def test_setitem_scalar_1d(arr, idx, val): arr[idx] = val assert np.array_equal(arr, [-1]) From 2f0da67621d3960031217b204af6b9af9feda21a Mon Sep 17 00:00:00 2001 From: Manolis Papadakis Date: Wed, 3 Aug 2022 14:40:23 -0700 Subject: [PATCH 8/8] Add new required test packages to conda env files --- conda/environment-test-3.10.yml | 1 + conda/environment-test-3.8.yml | 1 + conda/environment-test-3.9.yml | 1 + 3 files changed, 3 insertions(+) diff --git a/conda/environment-test-3.10.yml b/conda/environment-test-3.10.yml index 1e4652291..7ff2fdcef 100644 --- a/conda/environment-test-3.10.yml +++ b/conda/environment-test-3.10.yml @@ -29,6 +29,7 @@ dependencies: - pynvml - pytest - pytest-cov + - pytest-lazy-fixture - types-docutils # pip dependencies diff --git a/conda/environment-test-3.8.yml b/conda/environment-test-3.8.yml index 7026867cb..b4874cab6 100644 --- a/conda/environment-test-3.8.yml +++ b/conda/environment-test-3.8.yml @@ -29,6 +29,7 @@ dependencies: - pynvml - pytest - pytest-cov + - pytest-lazy-fixture - types-docutils # pip dependencies diff --git a/conda/environment-test-3.9.yml b/conda/environment-test-3.9.yml index 8b4660279..86f7b72fe 100644 --- a/conda/environment-test-3.9.yml +++ b/conda/environment-test-3.9.yml @@ -29,6 +29,7 @@ dependencies: - pynvml - pytest - pytest-cov + - pytest-lazy-fixture - types-docutils # pip dependencies