Native astype #642

Rubtsowa · 2021-03-09T18:29:45Z

No description provided.

…ive_astype

samir-nasibli

Provide also info about tests, that you are disabling.

samir-nasibli · 2021-03-09T19:04:32Z

dpnp/backend/kernels/dpnp_krnl_common.cpp

@@ -35,6 +35,42 @@
 namespace mkl_blas = oneapi::mkl::blas;
 namespace mkl_lapack = oneapi::mkl::lapack;

+template <typename _DataType, typename _TypeNew, typename _ResultType>


what is _TypeNew for?

samir-nasibli · 2021-03-09T19:07:07Z

dpnp/backend/kernels/dpnp_krnl_common.cpp

+class dpnp_astype_c_kernel;
+
+template <typename _DataType, typename _TypeNew, typename _ResultType>
+void dpnp_astype_c(void* array1_in, void* result1, size_t size)


remember this interface only for unsafe casting kind.

samir-nasibli · 2021-03-09T19:34:29Z

dpnp/backend/kernels/dpnp_krnl_common.cpp

+void dpnp_astype_c(void* array1_in, void* result1, size_t size)
+{
+    cl::sycl::event event;
+
+    _DataType* array_in = reinterpret_cast<_DataType*>(array1_in);
+    _ResultType* result = reinterpret_cast<_ResultType*>(result1);


Suggested change

void dpnp_astype_c(void* array1_in, void* result1, size_t size)

{

cl::sycl::event event;

_DataType* array_in = reinterpret_cast<_DataType*>(array1_in);

_ResultType* result = reinterpret_cast<_ResultType*>(result1);

void dpnp_astype_c(const void* array1_in, void* result1, const size_t size)

{

cl::sycl::event event;

const _DataType* array_in = reinterpret_cast<const _DataType*>(array1_in);

_ResultType* result = reinterpret_cast<_ResultType*>(result1);

Try not to ignore suggestions from previous PRs.

samir-nasibli · 2021-03-09T19:41:40Z

dpnp/dparray.pyx

+        if (not use_origin_backend(self)):
+            if not isinstance(self, dparray):
+                pass
+            elif casting is not None:
+                pass
+            elif subok is not None:
+                pass
+            elif copy is not True:
+                pass
+            elif order is not 'K':
+                pass
+            else:
+                return dpnp_astype(self, dtype)

-        return dpnp_astype(self, dtype)
+        return call_origin(numpy.ndarray.astype, self, dtype, order, casting, subok, copy)


I think doing fallback on numpy.ndarray here is bad idea.
It is better just throw error.

samir-nasibli · 2021-03-09T19:44:41Z

tests_external/skipped_tests_numpy.tbl

@@ -2881,6 +2881,7 @@ tests/test_umath.py::test_rint_big_int
 tests/test_umath.py::TestRoundingFunctions::test_object_direct
 tests/test_umath.py::TestRoundingFunctions::test_object_indirect
 tests/test_umath.py::test_signaling_nan_exceptions
+tests/test_umath.py::TestSign::test_sign_dtype_nan_object


what is the reason?
Provide info on description, please.

This test failed with error:
DPNP error: in function dpnp_dtype_to_DPNPFuncType() type '<class 'object'>' is not supported

object is a (global) variable. By default it is bound to a built-in class which is the root of the type hierarchy.

dpnp_dtype_to_DPNPFuncType can not convert object to dpnpc function types (DPNP_FT_DOUBLE, DPNP_FT_FLOAT, DPNP_FT_LONG, ...).
See this internal func impl here.

You can fallback on numpy for case when dtype is object.

You can fallback on numpy for case when dtype is object.

Sorry, my fault. It is bad proposal, just leave it as is.
No need doing fallback on numpy for dparray own methods.

samir-nasibli · 2021-03-09T19:45:41Z

tests/test_dparray.py

+@pytest.mark.parametrize("arr_dtype",
+                         [numpy.float64, numpy.float32, numpy.int64, numpy.int32, numpy.bool, numpy.bool_, numpy.complex],
+                         ids=['float64', 'float32', 'int64', 'int32', 'bool', 'bool_', 'complex'])
+@pytest.mark.parametrize("arr",


what if array is empty?
Corner cases.

samir-nasibli · 2021-03-09T19:48:34Z

tests/skipped_tests_gpu.tbl

+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-float64]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-float32]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-int64]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-int32]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-bool]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-bool_]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-complex]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-float64]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-float32]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-int64]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-int32]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-bool]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-bool_]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-complex]


please, provide some info why are you skipping all this tests?

samir-nasibli · 2021-03-09T19:49:18Z

tests/skipped_tests.tbl

+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-float64]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-float32]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-int64]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-int32]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-bool]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-bool_]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-complex]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-float64]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-float32]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-int64]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-int32]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-bool]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-bool_]
+tests/test_dparray.py::test_astype[[[-2, -1], [1, 2]]-complex-complex]


what is the reason?

because

error: assigning to 'bool' from incompatible type 'const std::complex'
error: assigning to 'int' from incompatible type 'const std::complex'
error: assigning to 'long' from incompatible type 'const std::complex'
error: assigning to 'float' from incompatible type 'const std::complex'
error: assigning to 'double' from incompatible type 'const std::complex'

Seems like they have the same issue. Investigation and fix is needed.

Seems like they have the same issue. Investigation and fix is needed.

This can be done on separate PR.

samir-nasibli · 2021-03-10T12:06:44Z

So seems that, you are disabling even more tests for changes proposed by you.
You need to investigate them.

Rubtsowa · 2021-03-10T12:28:04Z

@samir-nasibli you sure that I am disabling even more tests?
I added 196 tests, 20 of them skipped(not converted many types in complex in c++ ). And I uskip 1 test and 1 skip in existing tests.

samir-nasibli · 2021-03-10T13:00:44Z

tests/skipped_tests_gpu.tbl

+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-int32]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-bool]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-bool_]
+tests/test_dparray.py::test_astype[[-2, -1, 0, 1, 2]-complex-complex]


Complex to complex doesn't works?

samir-nasibli · 2021-03-10T22:02:13Z

tests/skipped_tests.tbl

@@ -154,7 +168,6 @@ tests/test_linalg.py::test_svd[(5,3)-complex128]
 tests/test_random.py::TestPermutationsTestShuffle::test_shuffle1[ lambda x: x]
 tests/test_random.py::TestPermutationsTestShuffle::test_shuffle1[ lambda x: dpnp.asarray(x).astype(dpnp.int8)]
 tests/test_random.py::TestPermutationsTestShuffle::test_shuffle1[ lambda x: dpnp.asarray(x).astype(dpnp.complex64)]
-tests/test_random.py::TestPermutationsTestShuffle::test_shuffle1[ lambda x: dpnp.asarray(x).astype(object)]


samir-nasibli

Looks pretty good. Thanks!

samir-nasibli · 2021-03-10T22:05:27Z

tests/test_dparray.py

+@pytest.mark.parametrize("arr_dtype",
+                         [numpy.float64, numpy.float32, numpy.int64, numpy.int32, numpy.bool, numpy.bool_, numpy.complex],
+                         ids=['float64', 'float32', 'int64', 'int32', 'bool', 'bool_', 'complex'])
+@pytest.mark.parametrize("arr",


samir-nasibli · 2021-03-10T22:10:55Z

Issues with complex type can be solved on other PR.

shssf · 2021-03-11T01:53:45Z

dpnp/dpnp_algo/dpnp_algo.pyx

@@ -125,10 +126,16 @@ cpdef dparray dpnp_array(obj, dtype=None):


 cpdef dparray dpnp_astype(dparray array1, dtype_target):
-    cdef dparray result = dparray(array1.shape, dtype=dtype_target)
+    cdef DPNPFuncType param1_type = dpnp_dtype_to_DPNPFuncType(array1.dtype)
+    cdef DPNPFuncType param2_type = dpnp_dtype_to_DPNPFuncType(dtype_target)


dtype_target might be diffrent. It can contain some object with no dtype. I think this is the cause of the error you saw

This PR suggests adding a temporary workaround to a problem in oneMath [#642 ](uxlfoundation/oneMath#642) where exceptions are no longer thrown in `lapack` namespace for `getrf` function as expected. In oneMath develop branch `oneapi::mkl::lapack::computation_error` is not thrown. Instead, `oneapi::mkl::computation_error` from `mkl` namespace is used so existing catch block `mkl_lapack::exception` does not handle singular matrix errors. A workaround has been added to explicitly catch `oneapi::mkl::computation_error` and update `dev_info` ensuring that singular matrices are handled correctly.

This PR suggests adding a temporary workaround to a problem in oneMath [#642 ](uxlfoundation/oneMath#642) where exceptions are no longer thrown in `lapack` namespace for `getrf` function as expected. In oneMath develop branch `oneapi::mkl::lapack::computation_error` is not thrown. Instead, `oneapi::mkl::computation_error` from `mkl` namespace is used so existing catch block `mkl_lapack::exception` does not handle singular matrix errors. A workaround has been added to explicitly catch `oneapi::mkl::computation_error` and update `dev_info` ensuring that singular matrices are handled correctly. 1b0ce60

Rubtsowa added 3 commits March 3, 2021 18:43

start change

f6eb548

Merge branch 'master' of https://github.com/IntelPython/dpnp into nat…

04f4764

…ive_astype

add kernel dpnp.astype()

af02a37

Rubtsowa requested a review from shssf March 9, 2021 18:29

samir-nasibli suggested changes Mar 9, 2021

View reviewed changes

add tests & change impl

e75921d

samir-nasibli reviewed Mar 10, 2021

View reviewed changes

unskip some tests & delete blank lines & add line in type_map

e3c4fe5

samir-nasibli reviewed Mar 10, 2021

View reviewed changes

samir-nasibli approved these changes Mar 10, 2021

View reviewed changes

shssf reviewed Mar 11, 2021

View reviewed changes

shssf approved these changes Mar 11, 2021

View reviewed changes

shssf merged commit 299667d into IntelPython:master Mar 11, 2021

Rubtsowa deleted the native_astype branch March 11, 2021 09:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Native astype #642

Native astype #642

Rubtsowa commented Mar 9, 2021

samir-nasibli left a comment

samir-nasibli Mar 9, 2021

samir-nasibli Mar 9, 2021

samir-nasibli Mar 9, 2021

samir-nasibli Mar 9, 2021

samir-nasibli Mar 9, 2021

Rubtsowa Mar 10, 2021

samir-nasibli Mar 10, 2021 •

edited

Loading

samir-nasibli Mar 10, 2021

samir-nasibli Mar 9, 2021

samir-nasibli Mar 10, 2021

samir-nasibli Mar 9, 2021

samir-nasibli Mar 9, 2021

Rubtsowa Mar 10, 2021

samir-nasibli Mar 10, 2021

samir-nasibli Mar 10, 2021

samir-nasibli commented Mar 10, 2021

Rubtsowa commented Mar 10, 2021

samir-nasibli Mar 10, 2021

samir-nasibli Mar 10, 2021 •

edited

Loading

samir-nasibli left a comment

samir-nasibli Mar 10, 2021

samir-nasibli commented Mar 10, 2021

shssf Mar 11, 2021

Native astype #642

Native astype #642

Conversation

Rubtsowa commented Mar 9, 2021

samir-nasibli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samir-nasibli Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samir-nasibli commented Mar 10, 2021

Rubtsowa commented Mar 10, 2021

Choose a reason for hiding this comment

samir-nasibli Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

samir-nasibli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samir-nasibli commented Mar 10, 2021

Choose a reason for hiding this comment

samir-nasibli Mar 10, 2021 •

edited

Loading

samir-nasibli Mar 10, 2021 •

edited

Loading