From cb846e83345764dd229dcfed04ad174e8c47ff38 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Thu, 2 Sep 2021 20:05:39 -0500 Subject: [PATCH 1/3] Reworked selector string handling, using str, rather than unicode Fixed a bug in memory operation methods of SyclQueue which went unnoticed for a long time since it was not covered by tests. --- dpctl/_sycl_queue.pyx | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/dpctl/_sycl_queue.pyx b/dpctl/_sycl_queue.pyx index c604cda850..945fee6f82 100644 --- a/dpctl/_sycl_queue.pyx +++ b/dpctl/_sycl_queue.pyx @@ -289,18 +289,13 @@ cdef class SyclQueue(_SyclQueue): status = self._init_queue_default(props) elif len_args == 1: arg = args[0] - if type(arg) is unicode: - string = bytes(arg, "utf-8") + if type(arg) is str: + string = bytes(arg, "utf-8") filter_c_str = string status = self._init_queue_from_filter_string( filter_c_str, props) elif type(arg) is _SyclQueue: status = self._init_queue_from__SyclQueue(<_SyclQueue>arg) - elif isinstance(arg, unicode): - string = bytes(unicode(arg), "utf-8") - filter_c_str = string - status = self._init_queue_from_filter_string( - filter_c_str, props) elif isinstance(arg, SyclDevice): status = self._init_queue_from_device(arg, props) elif pycapsule.PyCapsule_IsValid(arg, "SyclQueueRef"): @@ -690,7 +685,7 @@ cdef class SyclQueue(_SyclQueue): The address of the ``DPCTLSyclQueueRef`` object used to create this :class:`dpctl.SyclQueue` cast to a ``size_t``. """ - return int(self._queue_ref) + return self._queue_ref cpdef SyclEvent submit( self, @@ -848,8 +843,8 @@ cdef class SyclQueue(_SyclQueue): else: raise TypeError("Parameter `mem` should have type _Memory") - if (count <=0 or count > self.nbytes): - count = self.nbytes + if (count <=0 or count > mem.nbytes): + count = mem.nbytes ERef = DPCTLQueue_Prefetch(self._queue_ref, ptr, count) if (ERef is NULL): @@ -868,8 +863,8 @@ cdef class SyclQueue(_SyclQueue): else: raise TypeError("Parameter `mem` should have type _Memory") - if (count <=0 or count > self.nbytes): - count = self.nbytes + if (count <=0 or count > mem.nbytes): + count = mem.nbytes ERef = DPCTLQueue_MemAdvise(self._queue_ref, ptr, count, advice) if (ERef is NULL): From c1895f7184ff1d55cc79892d7da2af2196f01b14 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Thu, 2 Sep 2021 20:06:59 -0500 Subject: [PATCH 2/3] Extended tests to improve coverage --- dpctl/tests/test_sycl_context.py | 23 +++++++++++ dpctl/tests/test_sycl_queue.py | 66 +++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/dpctl/tests/test_sycl_context.py b/dpctl/tests/test_sycl_context.py index e624c83d99..c938caf074 100644 --- a/dpctl/tests/test_sycl_context.py +++ b/dpctl/tests/test_sycl_context.py @@ -187,3 +187,26 @@ def test_hashing_of_context(): def test_context_repr(): ctx = dpctl.SyclContext() assert type(ctx.__repr__()) is str + + +def test_cpython_api(): + import ctypes + import sys + + ctx = dpctl.SyclContext() + mod = sys.modules[ctx.__class__.__module__] + # get capsule storign get_context_ref function ptr + ctx_ref_fn_cap = mod.__pyx_capi__["get_context_ref"] + # construct Python callable to invoke "get_context_ref" + cap_ptr_fn = ctypes.pythonapi.PyCapsule_GetPointer + cap_ptr_fn.restype = ctypes.c_void_p + cap_ptr_fn.argtypes = [ctypes.py_object, ctypes.c_char_p] + ctx_ref_fn_ptr = cap_ptr_fn( + ctx_ref_fn_cap, b"DPCTLSyclContextRef (struct PySyclContextObject *)" + ) + callable_maker = ctypes.PYFUNCTYPE(ctypes.c_void_p, ctypes.py_object) + get_context_ref_fn = callable_maker(ctx_ref_fn_ptr) + + r2 = ctx.addressof_ref() + r1 = get_context_ref_fn(ctx) + assert r1 == r2 diff --git a/dpctl/tests/test_sycl_queue.py b/dpctl/tests/test_sycl_queue.py index 655edff06a..941746652c 100644 --- a/dpctl/tests/test_sycl_queue.py +++ b/dpctl/tests/test_sycl_queue.py @@ -433,16 +433,80 @@ def test_queue__repr__(): r2 = q2.__repr__() q3 = dpctl.SyclQueue(property="enable_profiling") r3 = q3.__repr__() - q4 = dpctl.SyclQueue(property=["in_order", "enable_profiling"]) + q4 = dpctl.SyclQueue(property="default") r4 = q4.__repr__() + q5 = dpctl.SyclQueue(property=["in_order", "enable_profiling"]) + r5 = q5.__repr__() assert type(r1) is str assert type(r2) is str assert type(r3) is str assert type(r4) is str + assert type(r5) is str + + +def test_queue_invalid_property(): + with pytest.raises(ValueError): + dpctl.SyclQueue(property=4.5) + with pytest.raises(ValueError): + dpctl.SyclQueue(property=["abc", tuple()]) def test_queue_capsule(): q = dpctl.SyclQueue() cap = q._get_capsule() + cap2 = q._get_capsule() q2 = dpctl.SyclQueue(cap) assert q == q2 + del cap2 # call deleter on non-renamed capsule + + +def test_cpython_api(): + import ctypes + import sys + + q = dpctl.SyclQueue() + mod = sys.modules[q.__class__.__module__] + # get capsule storign get_context_ref function ptr + q_ref_fn_cap = mod.__pyx_capi__["get_queue_ref"] + # construct Python callable to invoke "get_queue_ref" + cap_ptr_fn = ctypes.pythonapi.PyCapsule_GetPointer + cap_ptr_fn.restype = ctypes.c_void_p + cap_ptr_fn.argtypes = [ctypes.py_object, ctypes.c_char_p] + q_ref_fn_ptr = cap_ptr_fn( + q_ref_fn_cap, b"DPCTLSyclQueueRef (struct PySyclQueueObject *)" + ) + callable_maker = ctypes.PYFUNCTYPE(ctypes.c_void_p, ctypes.py_object) + get_queue_ref_fn = callable_maker(q_ref_fn_ptr) + + r2 = q.addressof_ref() + r1 = get_queue_ref_fn(q) + assert r1 == r2 + + +def test_constructor_many_arg(): + with pytest.raises(TypeError): + dpctl.SyclQueue(None, None, None, None) + with pytest.raises(TypeError): + dpctl.SyclQueue(None, None) + + +def test_queue_wait(): + try: + q = dpctl.SyclQueue() + except dpctl.SyclQueueCreationError: + pytest.skip("Failed to create device with supported filter") + q.wait() + + +def test_queue_memops(): + try: + q = dpctl.SyclQueue() + except dpctl.SyclQueueCreationError: + pytest.skip("Failed to create device with supported filter") + from dpctl.memory import MemoryUSMDevice + + m1 = MemoryUSMDevice(512, queue=q) + m2 = MemoryUSMDevice(512, queue=q) + q.memcpy(m1, m2, 512) + q.prefetch(m1, 512) + q.mem_advise(m1, 512, 0) From c5ed2a71d24935d07c87a4b1d9a26ca60863a037 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Fri, 3 Sep 2021 07:02:59 -0500 Subject: [PATCH 3/3] Renamed get_sycl_backend method in SyclQueue to backend property Removed name-sake property added to query the underlying sycl_device --- dpctl/_sycl_queue.pyx | 18 ++++++------------ dpctl/tests/test_sycl_queue.py | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/dpctl/_sycl_queue.pyx b/dpctl/_sycl_queue.pyx index 945fee6f82..de719de049 100644 --- a/dpctl/_sycl_queue.pyx +++ b/dpctl/_sycl_queue.pyx @@ -644,8 +644,12 @@ cdef class SyclQueue(_SyclQueue): else: return False - def get_sycl_backend(self): - """ Returns the Sycl backend associated with the queue. + @property + def backend(self): + """ Returns the backend_type enum value for this queue. + + Returns: + backend_type: The backend for the queue. """ cdef _backend_type BE = DPCTLQueue_GetBackend(self._queue_ref) if BE == _backend_type._OPENCL: @@ -959,16 +963,6 @@ cdef class SyclQueue(_SyclQueue): return SyclEvent._create(ERef, []) - @property - def backend(self): - """Returns the backend_type enum value for the device - associated with this queue. - - Returns: - backend_type: The backend for the device. - """ - return self.sycl_device.backend - @property def name(self): """Returns the device name for the device diff --git a/dpctl/tests/test_sycl_queue.py b/dpctl/tests/test_sycl_queue.py index 941746652c..5b978153ae 100644 --- a/dpctl/tests/test_sycl_queue.py +++ b/dpctl/tests/test_sycl_queue.py @@ -424,6 +424,8 @@ def test_queue_submit_barrier(valid_filter): ev3.wait() ev1.wait() ev2.wait() + with pytest.raises(TypeError): + q.submit_barrier(range(3)) def test_queue__repr__(): @@ -488,6 +490,11 @@ def test_constructor_many_arg(): dpctl.SyclQueue(None, None, None, None) with pytest.raises(TypeError): dpctl.SyclQueue(None, None) + ctx = dpctl.SyclContext() + with pytest.raises(TypeError): + dpctl.SyclQueue(ctx, None) + with pytest.raises(TypeError): + dpctl.SyclQueue(ctx) def test_queue_wait(): @@ -510,3 +517,11 @@ def test_queue_memops(): q.memcpy(m1, m2, 512) q.prefetch(m1, 512) q.mem_advise(m1, 512, 0) + with pytest.raises(TypeError): + q.memcpy(m1, list(), 512) + with pytest.raises(TypeError): + q.memcpy(list(), m2, 512) + with pytest.raises(TypeError): + q.prefetch(list(), 512) + with pytest.raises(TypeError): + q.mem_advise(list(), 512, 0)