Skip to content

Commit 1ca7191

Browse files
antonwolfyoleksandr-pavlykxaleryb
authored
Merge changes for nightly tests in CI (#1173)
* dpnp_take_c uses SYCL kernel, no need to use no_sycl parameter in adapter The reason this crashed with CPU device and gave incorrect results on Windows was deeper. 1. Adapter call allocated USM-shared buffer and copies data into it 2. Kernel is submitted to work on USM-shared pointer 3. dpnp_take_c returns kernel submission even 4. Adapter class goes out of scope and frees USM allocation without making sure that the kernel that works on it has completed its execution 5. If kernel execution was in progress we got a crash on CPU, or incorrect result on GPU If kernel execution was complete it worked as expected. This change fixes the problem because it removes creation of unprotected USM-shared temporary. * Change to DPNPC_adapter to set/use events upon which deallocation must depend The deallocation routine simply calls sycl::event::wait on the stored vector. * Used DPNPC_ptr_adapter::depends_on Also applied DPCTLEvent_Delete in legacy interfaces to avoid memory leak. * Get rid of "Improper Null Termination" issue Add a null-terminated symbol at the end of char array to avoid "Improper Null Termination" issue reported by Checkmarx scan. * implemented PR feedback * Reworked solution with a pointer on void * Update dpnp/backend/kernels/dpnp_krnl_random.cpp Co-authored-by: Oleksandr Pavlyk <[email protected]> * Update dpnp/backend/kernels/dpnp_krnl_random.cpp Co-authored-by: Oleksandr Pavlyk <[email protected]> * Skip for two more tests till waiting fix (#1171) * Skip for two more tests till waiting fix tests/third_party/cupy/statistics_tests/test_order.py::TestOrder::test_ptp_all_nan tests/third_party/cupy/statistics_tests/test_order.py::TestOrder::test_ptp_nan Need to skip them because CI does not work due to this. * The same tests skip for gpu * dpnp_take failed on Windows due to memory corruption (#1172) * dpnp_take failed on Windows due to memory corruption * Add more tests * Integer indexes types with different types of input data * Add trailing empty line to .gitignore Co-authored-by: Oleksandr Pavlyk <[email protected]> Co-authored-by: Alexander Rybkin <[email protected]>
1 parent 4102660 commit 1ca7191

File tree

4 files changed

+49
-35
lines changed

4 files changed

+49
-35
lines changed

.gitignore

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
1+
# CMake build and local install directory
12
build
23
build_cython
4+
5+
# Byte-compiled / optimized / DLL files
36
__pycache__/
7+
8+
# Code project files
9+
.vscode
10+
411
*dpnp_backend*
512
dpnp/**/*.cpython*.so
613
dpnp/**/*.pyd
7-
*~
14+
*~
15+
core

dpnp/backend/kernels/dpnp_krnl_indexing.cpp

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -901,10 +901,8 @@ DPCTLSyclEventRef dpnp_take_c(DPCTLSyclQueueRef q_ref,
901901
DPCTLSyclEventRef event_ref = nullptr;
902902
sycl::queue q = *(reinterpret_cast<sycl::queue*>(q_ref));
903903

904-
DPNPC_ptr_adapter<_DataType> input1_ptr(q_ref, array1_in, array1_size);
905-
DPNPC_ptr_adapter<_IndecesType> input2_ptr(q_ref, indices1, size);
906-
_DataType* array_1 = input1_ptr.get_ptr();
907-
_IndecesType* indices = input2_ptr.get_ptr();
904+
_DataType* array_1 = reinterpret_cast<_DataType*>(array1_in);
905+
_IndecesType* indices = reinterpret_cast<_IndecesType*>(indices1);
908906
_DataType* result = reinterpret_cast<_DataType*>(result1);
909907

910908
sycl::range<1> gws(size);
@@ -920,7 +918,6 @@ DPCTLSyclEventRef dpnp_take_c(DPCTLSyclQueueRef q_ref,
920918
sycl::event event = q.submit(kernel_func);
921919

922920
event_ref = reinterpret_cast<DPCTLSyclEventRef>(&event);
923-
924921
return DPCTLEvent_Copy(event_ref);
925922
}
926923

@@ -937,6 +934,7 @@ void dpnp_take_c(void* array1_in, const size_t array1_size, void* indices1, void
937934
size,
938935
dep_event_vec_ref);
939936
DPCTLEvent_WaitAndThrow(event_ref);
937+
DPCTLEvent_Delete(event_ref);
940938
}
941939

942940
template <typename _DataType, typename _IndecesType>
@@ -1073,21 +1071,36 @@ void func_map_init_indexing_func(func_map_t& fmap)
10731071
fmap[DPNPFuncName::DPNP_FN_PUT_ALONG_AXIS_EXT][eft_DBL][eft_DBL] = {eft_DBL,
10741072
(void*)dpnp_put_along_axis_ext_c<double>};
10751073

1076-
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_BLN][eft_BLN] = {eft_BLN, (void*)dpnp_take_default_c<bool, int64_t>};
1077-
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_INT][eft_INT] = {eft_INT, (void*)dpnp_take_default_c<int32_t, int64_t>};
1074+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_BLN][eft_INT] = {eft_BLN, (void*)dpnp_take_default_c<bool, int32_t>};
1075+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_INT][eft_INT] = {eft_INT, (void*)dpnp_take_default_c<int32_t, int32_t>};
1076+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_LNG][eft_INT] = {eft_LNG, (void*)dpnp_take_default_c<int64_t, int32_t>};
1077+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_FLT][eft_INT] = {eft_FLT, (void*)dpnp_take_default_c<float, int32_t>};
1078+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_DBL][eft_INT] = {eft_DBL, (void*)dpnp_take_default_c<double, int32_t>};
1079+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_C128][eft_INT] = {eft_C128,
1080+
(void*)dpnp_take_default_c<std::complex<double>, int32_t>};
1081+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_BLN][eft_LNG] = {eft_BLN, (void*)dpnp_take_default_c<bool, int64_t>};
1082+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_INT][eft_LNG] = {eft_INT, (void*)dpnp_take_default_c<int32_t, int32_t>};
10781083
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_LNG][eft_LNG] = {eft_LNG, (void*)dpnp_take_default_c<int64_t, int64_t>};
1079-
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_FLT][eft_FLT] = {eft_FLT, (void*)dpnp_take_default_c<float, int64_t>};
1080-
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_DBL][eft_DBL] = {eft_DBL, (void*)dpnp_take_default_c<double, int64_t>};
1081-
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_C128][eft_C128] = {eft_C128,
1082-
(void*)dpnp_take_default_c<std::complex<double>, int64_t>};
1083-
1084-
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_BLN][eft_BLN] = {eft_BLN, (void*)dpnp_take_ext_c<bool, int64_t>};
1085-
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_INT][eft_INT] = {eft_INT, (void*)dpnp_take_ext_c<int32_t, int64_t>};
1084+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_FLT][eft_LNG] = {eft_FLT, (void*)dpnp_take_default_c<float, int64_t>};
1085+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_DBL][eft_LNG] = {eft_DBL, (void*)dpnp_take_default_c<double, int64_t>};
1086+
fmap[DPNPFuncName::DPNP_FN_TAKE][eft_C128][eft_LNG] = {eft_C128,
1087+
(void*)dpnp_take_default_c<std::complex<double>, int64_t>};
1088+
1089+
// TODO: add a handling of other indexes types once DPCtl implementation of data copy is ready
1090+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_BLN][eft_INT] = {eft_BLN, (void*)dpnp_take_ext_c<bool, int32_t>};
1091+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_INT][eft_INT] = {eft_INT, (void*)dpnp_take_ext_c<int32_t, int32_t>};
1092+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_LNG][eft_INT] = {eft_LNG, (void*)dpnp_take_ext_c<int64_t, int32_t>};
1093+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_FLT][eft_INT] = {eft_FLT, (void*)dpnp_take_ext_c<float, int32_t>};
1094+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_DBL][eft_INT] = {eft_DBL, (void*)dpnp_take_ext_c<double, int32_t>};
1095+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_C128][eft_INT] = {eft_C128,
1096+
(void*)dpnp_take_ext_c<std::complex<double>, int32_t>};
1097+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_BLN][eft_LNG] = {eft_BLN, (void*)dpnp_take_ext_c<bool, int64_t>};
1098+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_INT][eft_LNG] = {eft_INT, (void*)dpnp_take_ext_c<int32_t, int64_t>};
10861099
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_LNG][eft_LNG] = {eft_LNG, (void*)dpnp_take_ext_c<int64_t, int64_t>};
1087-
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_FLT][eft_FLT] = {eft_FLT, (void*)dpnp_take_ext_c<float, int64_t>};
1088-
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_DBL][eft_DBL] = {eft_DBL, (void*)dpnp_take_ext_c<double, int64_t>};
1089-
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_C128][eft_C128] = {eft_C128,
1090-
(void*)dpnp_take_ext_c<std::complex<double>, int64_t>};
1100+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_FLT][eft_LNG] = {eft_FLT, (void*)dpnp_take_ext_c<float, int64_t>};
1101+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_DBL][eft_LNG] = {eft_DBL, (void*)dpnp_take_ext_c<double, int64_t>};
1102+
fmap[DPNPFuncName::DPNP_FN_TAKE_EXT][eft_C128][eft_LNG] = {eft_C128,
1103+
(void*)dpnp_take_ext_c<std::complex<double>, int64_t>};
10911104

10921105
return;
10931106
}

tests/skipped_tests_gpu.tbl

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -138,19 +138,6 @@ tests/test_indexing.py::test_nonzero[[[0, 1, 2], [3, 0, 5], [6, 7, 0]]]
138138
tests/test_indexing.py::test_nonzero[[[0, 1, 0, 3, 0], [5, 0, 7, 0, 9]]]
139139
tests/test_indexing.py::test_nonzero[[[[1, 2], [0, 4]], [[0, 2], [0, 1]], [[0, 0], [3, 1]]]]
140140
tests/test_indexing.py::test_nonzero[[[[[1, 2, 3], [3, 4, 5]], [[1, 2, 3], [2, 1, 0]]], [[[1, 3, 5], [3, 1, 0]], [[0, 1, 2], [1, 3, 4]]]]]
141-
tests/test_indexing.py::test_take[[[0, 1, 2], [3, 4, 5], [6, 7, 8]]-[[0, 0], [0, 0]]]
142-
tests/test_indexing.py::test_take[[[0, 1, 2], [3, 4, 5], [6, 7, 8]]-[[1, 2], [1, 2]]]
143-
tests/test_indexing.py::test_take[[[0, 1, 2], [3, 4, 5], [6, 7, 8]]-[[1, 2], [3, 4]]]
144-
tests/test_indexing.py::test_take[[[0, 1, 2, 3, 4], [5, 6, 7, 8, 9]]-[[1, 2], [1, 2]]]
145-
tests/test_indexing.py::test_take[[[0, 1, 2, 3, 4], [5, 6, 7, 8, 9]]-[[1, 2], [3, 4]]]
146-
tests/test_indexing.py::test_take[[[[1, 2], [3, 4]], [[1, 2], [2, 1]], [[1, 3], [3, 1]]]-[[1, 2], [1, 2]]]
147-
tests/test_indexing.py::test_take[[[[1, 2], [3, 4]], [[1, 2], [2, 1]], [[1, 3], [3, 1]]]-[[1, 2], [3, 4]]]
148-
tests/test_indexing.py::test_take[[[[[1, 2], [3, 4]], [[1, 2], [2, 1]]], [[[1, 3], [3, 1]], [[0, 1], [1, 3]]]]-[[1, 2], [1, 2]]]
149-
tests/test_indexing.py::test_take[[[[[1, 2], [3, 4]], [[1, 2], [2, 1]]], [[[1, 3], [3, 1]], [[0, 1], [1, 3]]]]-[[1, 2], [3, 4]]]
150-
tests/test_indexing.py::test_take[[[[[1, 2, 3], [3, 4, 5]], [[1, 2, 3], [2, 1, 0]]], [[[1, 3, 5], [3, 1, 0]], [[0, 1, 2], [1, 3, 4]]]]-[[1, 2], [1, 2]]]
151-
tests/test_indexing.py::test_take[[[[[1, 2, 3], [3, 4, 5]], [[1, 2, 3], [2, 1, 0]]], [[[1, 3, 5], [3, 1, 0]], [[0, 1, 2], [1, 3, 4]]]]-[[1, 2], [3, 4]]]
152-
tests/test_indexing.py::test_take[[[[[1, 2, 3], [4, 5, 6]], [[7, 8, 9], [10, 11, 12]]], [[[13, 14, 15], [16, 17, 18]], [[19, 20, 21], [22, 23, 24]]]]-[[1, 2], [1, 2]]]
153-
tests/test_indexing.py::test_take[[[[[1, 2, 3], [4, 5, 6]], [[7, 8, 9], [10, 11, 12]]], [[[13, 14, 15], [16, 17, 18]], [[19, 20, 21], [22, 23, 24]]]]-[[1, 2], [3, 4]]]
154141
tests/third_party/cupy/creation_tests/test_ranges.py::TestRanges::test_arange_no_dtype_int
155142
tests/third_party/cupy/indexing_tests/test_indexing.py::TestIndexing::test_take_no_axis
156143
tests/third_party/cupy/indexing_tests/test_insert.py::TestPlace_param_3_{n_vals=1, shape=(7,)}::test_place

tests/test_indexing.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,12 @@ def test_select():
374374
numpy.testing.assert_array_equal(expected, result)
375375

376376

377+
@pytest.mark.parametrize("array_type",
378+
[numpy.bool8, numpy.int32, numpy.int64, numpy.float32, numpy.float64, numpy.complex128],
379+
ids=['bool8', 'int32', 'int64', 'float32', 'float64', 'complex128'])
380+
@pytest.mark.parametrize("indices_type",
381+
[numpy.int32, numpy.int64],
382+
ids=['int32', 'int64'])
377383
@pytest.mark.parametrize("indices",
378384
[[[0, 0], [0, 0]],
379385
[[1, 2], [1, 2]],
@@ -395,9 +401,9 @@ def test_select():
395401
'[[[[1, 2], [3, 4]], [[1, 2], [2, 1]]], [[[1, 3], [3, 1]], [[0, 1], [1, 3]]]]',
396402
'[[[[1, 2, 3], [3, 4, 5]], [[1, 2, 3], [2, 1, 0]]], [[[1, 3, 5], [3, 1, 0]], [[0, 1, 2], [1, 3, 4]]]]',
397403
'[[[[1, 2, 3], [4, 5, 6]], [[7, 8, 9], [10, 11, 12]]], [[[13, 14, 15], [16, 17, 18]], [[19, 20, 21], [22, 23, 24]]]]'])
398-
def test_take(array, indices):
399-
a = numpy.array(array)
400-
ind = numpy.array(indices)
404+
def test_take(array, indices, array_type, indices_type):
405+
a = numpy.array(array, dtype=array_type)
406+
ind = numpy.array(indices, dtype=indices_type)
401407
ia = dpnp.array(a)
402408
iind = dpnp.array(ind)
403409
expected = numpy.take(a, ind)

0 commit comments

Comments
 (0)