Subdevices work #326

1e-to · 2021-03-17T14:16:21Z

Closes #290

dpctl-capi/source/dpctl_sycl_device_interface.cpp

dpctl-capi/tests/test_sycl_device_interface.cpp

…b_devices

1e-to · 2021-03-22T14:35:04Z

2 notes being worked on

Need to change the vector in DPCTLDevice_CreateSubDevicesByCounts to what?
Add tests to DPCTLDevice_CreateSubDevicesByAffinity on all info :: partition_affinity_domain

diptorupd · 2021-03-22T15:51:08Z

* Need to change the vector in DPCTLDevice_CreateSubDevicesByCounts to what?

Yes, the pitfalls of our C API design. You can pass __dpctl_keep size_t* counts, size_t ncounts to the function.

dpctl-capi/source/dpctl_sycl_device_interface.cpp

dpctl-capi/helper/include/dpctl_utils_helper.h

…b_devices

dpctl-capi/source/dpctl_sycl_device_interface.cpp

Resolved conflicts by keep changes on both sides

…SubDevicesEqually and others

… cython

1e-to · 2021-03-26T17:39:13Z

Work on:

Find out a way how to convert python array to pointer in create_sub_devices_by_counts (May be use memset(a.data.as_voidptr, 0, len(a) * sizeof(int))?)
Make _partition_affinity_domain_type enum visible in python

1e-to · 2021-03-29T16:32:22Z

@diptorupd , @oleksandr-pavlyk could you please take a look and write what need to be changed?

oleksandr-pavlyk · 2021-03-30T20:28:12Z

@diptorupd , @oleksandr-pavlyk could you please take a look and write what need to be changed?

In [3]: d

Out[3]: <dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7ffa8d81f070>

In [4]: d.create_sub_devices(partition=6)  # returns None ?

This kind of worked:

In [8]: import numpy as np

In [9]: d.create_sub_devices_by_counts(np.array([2, 2, 2, 2, 2, 2], dtype=np.uint64), 6)
Out[9]:
[<dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7ffa9fa9a2f0>,
 <dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7ffa9fa9a5f0>,
 <dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7ffab44157b0>,
 <dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7ffab4415770>,
 <dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7ffab4415cb0>,
 <dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7ffab4415830>]

but it should be unnecessary to specify length 6 as the second argument. It also should not be needed to use numpy array for this. Using list should be supported, so I would have expected d.create_sub_devices_by_counts([2, 2, 2, 2, 2, 2]) to work.

create by partition should raise an error here, rather than return an empty list

In [12]: d.max_compute_units
Out[12]: 12

In [13]: d.create_sub_devices_equally(12)
Native API failed. Native API returns: -18 (CL_DEVICE_PARTITION_FAILED) -18 (CL_DEVICE_PARTITION_FAILED)
Out[13]: []

This worked fine.

In [14]: d.create_sub_devices_equally(6)
Out[14]:
[<dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7ffa8d0c48b0>,
 <dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7ffa8d0c4270>]

I thought we discussed that create_sub_devices_equally, create_sub_devices_by_partition and create_sub_devices_by_counts methods will be cdef only, and the only Python visible interface will be create_sub_devices(partition=...) interface.

diptorupd

Almost there. Looking good.

dpctl/_sycl_device.pyx

diptorupd · 2021-03-31T14:29:26Z

dpctl/_sycl_device.pyx

+        cdef DPCTLDeviceVectorRef DVRef = NULL
+        DVRef = DPCTLDevice_CreateSubDevicesByAffinity(self._device_ref, domain)
+        if DVRef is NULL:
+            self._raise_sub_devices_creation_error("DPCTLSubDeviceCreationError", -1)


@oleksandr-pavlyk Will Cython ever raise this exception? Should the cdef function have except + added to the signature?

@1e-to can you check if Cython raises this exception? Would be good to have a negative test case to see if an exception is raised.

Refer Sasha's comments in #35 (comment)

Yes, it raises

>>> d = dpctl.SyclDevice("cpu") >>> d.create_sub_devices("L2_cache") Traceback (most recent call last): File "<stdin>", line 1, in <module> File "dpctl/_sycl_device.pyx", line 268, in dpctl._sycl_device.SyclDevice.create_sub_devices return self.create_sub_devices_by_affinity(domain_type) File "dpctl/_sycl_device.pyx", line 241, in dpctl._sycl_device.SyclDevice.create_sub_devices_by_affinity self._raise_sub_devices_creation_error("DPCTLSubDeviceCreationError", -1) File "dpctl/_sycl_device.pyx", line 195, in dpctl._sycl_device.SyclDevice._raise_sub_devices_creation_error raise e dpctl._sycl_device.SubDeviceCreationError: Sub-devices were not created.

@1e-to Please add some negative test cases where we check if the exception is raised.

oleksandr-pavlyk · 2021-03-31T19:10:57Z

dpctl/_sycl_device.pyx

+        if DVRef is NULL:
+            self._raise_sub_devices_creation_error("DPCTLSubDeviceCreationError", -1)
+        cdef list devices = _get_devices(DVRef)
+        free(counts_buff)


if memory was allocated, but DVRef was NULL, the memory would never be freed.

I fixed this issue in #343

oleksandr-pavlyk · 2021-03-31T19:13:04Z

dpctl/_sycl_device.pyx

+        e = SubDeviceCreationError("Sub-devices were not created.")
+        e.fname = fname
+        e.code = errcode
+        raise e


This function serves no purpose. It is deleted in #343

Object e derives from Exception, which does not have attributes fname or code. Lines 193-194 dynamically add these attributes to the exception instance, but they go unused.

Calls self._raise_sub_devices_creation_error("DPCTLSubDeviceCreationError", -1) have been replaced with raise SubDeviceCreationError("Sub-devices were not created.") in #343

oleksandr-pavlyk · 2021-03-31T19:13:52Z

dpctl/_sycl_device.pyx

+            is created.
+        """
+        cdef size_t ncounts = len(counts)
+        cdef size_t *counts_buff = <size_t *> malloc(ncounts * sizeof(size_t))


Must check that pointer counts_buff is not NULL, and raise MemoryError if it is.

In any case, null pointer should not be dereferenced. Fixed in #343

oleksandr-pavlyk · 2021-03-31T19:14:51Z

dpctl/_sycl_device.pyx

+    def create_sub_devices(self, partition):
+        if isinstance(partition, int) and partition > 0:
+            return self.create_sub_devices_equally(partition)
+        elif isinstance(partition, list) and all([i > 0 for i in partition]):


all([i>0 for i in partition]) is redundant. It is being checked as part of conversion from element of partition to size_t on lines 223-224.

oleksandr-pavlyk · 2021-03-31T19:18:55Z

dpctl/_sycl_device.pyx

+            else:
+                raise Exception('Unsupported type of domain')
+            return self.create_sub_devices_by_affinity(domain_type)
+        else:


I reworked this dispatch in #343 to require keyword partition, so that device.create_sub_devices(4) must become device.create_sub_devices(partition=4) to work.

Changes allow allow for list/tuples/numpy arrays for specify partition by counts.

In [1]: import dpctl, numpy as np In [2]: [di.max_compute_units for di in dpctl.SyclDevice("cpu").create_sub_devices(partition=np.array([2, 6])) ] Out[2]: [2, 6] In [1]: import dpctl, numpy as np In [2]: [di.max_compute_units for di in dpctl.SyclDevice("cpu").create_sub_devices(partition=np.array([2, 6])) ] Out[2]: [2, 6] In [4]: [di.max_compute_units for di in dpctl.SyclDevice("cpu").create_sub_devices(partition = [2, 6, 4]) ] Out[4]: [2, 6, 4] In [5]: [di.max_compute_units for di in dpctl.SyclDevice("cpu").create_sub_devices(partition = (2, 6, 4)) ] Out[5]: [2, 6, 4]

Also use of NumPy integral types is supported:

In [6]: dpctl.SyclDevice("cpu").create_sub_devices(partition=np.int32(4)) Out[6]: [<dpctl.SyclDevice [backend_type.opencl, device_type.cpu, Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7f1f698727b0>, <dpctl.SyclDevice [backend_type.opencl, device_type.cpu, Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7f1f69872c30>, <dpctl.SyclDevice [backend_type.opencl, device_type.cpu, Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7f1f69872d30>]

oleksandr-pavlyk · 2021-04-02T16:04:21Z

Closing this PR, since the superseding PR #326 has been merged.

…on#326

wip

865ef9d

1e-to marked this pull request as draft March 17, 2021 14:18

diptorupd suggested changes Mar 19, 2021

View reviewed changes

dpctl-capi/source/dpctl_sycl_device_interface.cpp Show resolved Hide resolved

dpctl-capi/tests/test_sycl_device_interface.cpp Outdated Show resolved Hide resolved

etotmeni added 2 commits March 22, 2021 08:58

Added C-api initial for all sub-device funcs

1979b63

Merge branch 'master' of https://github.com/IntelPython/dpctl into su…

a1fa5f3

…b_devices

diptorupd mentioned this pull request Mar 22, 2021

Release dpctl 0.7.0 #325

Closed

8 tasks

Change vector type + Added tests

df04485

diptorupd suggested changes Mar 23, 2021

View reviewed changes

dpctl-capi/source/dpctl_sycl_device_interface.cpp Show resolved Hide resolved

dpctl-capi/source/dpctl_sycl_device_interface.cpp Show resolved Hide resolved

dpctl-capi/helper/include/dpctl_utils_helper.h Show resolved Hide resolved

diptorupd added this to the 0.7.0 milestone Mar 23, 2021

etotmeni added 3 commits March 24, 2021 04:49

Merge branch 'master' of https://github.com/IntelPython/dpctl into su…

3ff6620

…b_devices

fix tests + utils

6f4151d

Change vector to DeviceVectorRef

c7588e9

diptorupd reviewed Mar 24, 2021

View reviewed changes

dpctl-capi/source/dpctl_sycl_device_interface.cpp Outdated Show resolved Hide resolved

etotmeni and others added 11 commits March 24, 2021 10:25

Fix SetAt

30817fb

Backup

230a45b

Add backend

51f72d6

work on cython part

972f553

Added cpdef func

f7d2bfa

Merge remote-tracking branch 'origin/master' into sub_devices

8933f35

Resolved conflicts by keep changes on both sides

using const qualifier in iterating over vector of sub-devices

072305a

fix Cython error due to mismatch in declaration of DPCTLDevice_Create…

1838637

…SubDevicesEqually and others

Fix tests

4a5efc3

Delete commented code

ccebb86

Added create_sub_devices_by_counts and create_sub_devices_by_affinity…

d7ae52d

… cython

1e-to marked this pull request as ready for review March 26, 2021 17:41

etotmeni added 2 commits March 29, 2021 05:44

Merge master

64799c1

Fix cython create_sub_devices_by_counts

5130e69

etotmeni added 4 commits March 29, 2021 08:20

Fix create_sub_devices_by_affinity

3b4bae2

Add create_sub_devices for all funcs

77ba7d2

Del useless import

2f3e4fa

Fix formatting

b1aa862

1e-to changed the title ~~WIP subdevices work~~ Subdevices work Mar 29, 2021

diptorupd suggested changes Mar 31, 2021

View reviewed changes

etotmeni added 4 commits March 31, 2021 03:28

Merge master

f58b2c6

Small fixes

81df60e

Fix create_sub_devices_by_counts array

f3defb9

Del useless import

af10517

diptorupd reviewed Mar 31, 2021

View reviewed changes

oleksandr-pavlyk mentioned this pull request Mar 31, 2021

Review sub devices #343

Merged

oleksandr-pavlyk reviewed Mar 31, 2021

View reviewed changes

oleksandr-pavlyk pushed a commit that referenced this pull request Apr 2, 2021

dpctl-capi changes to add support for sub-devices from #326

943300a

oleksandr-pavlyk pushed a commit that referenced this pull request Apr 2, 2021

dpctl/ Python API change to add support for subdevices from #326

07eaae3

oleksandr-pavlyk closed this Apr 2, 2021

diptorupd pushed a commit to diptorupd/dpctl that referenced this pull request Apr 2, 2021

dpctl-capi changes to add support for sub-devices from IntelPython#326

ba2d023

diptorupd pushed a commit to diptorupd/dpctl that referenced this pull request Apr 2, 2021

dpctl/ Python API change to add support for subdevices from IntelPyth…

e6f90d6

…on#326

1e-to deleted the sub_devices branch November 3, 2021 09:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Subdevices work #326

Subdevices work #326

1e-to commented Mar 17, 2021

1e-to commented Mar 22, 2021

diptorupd commented Mar 22, 2021

1e-to commented Mar 26, 2021 •

edited

Loading

1e-to commented Mar 29, 2021

oleksandr-pavlyk commented Mar 30, 2021

diptorupd left a comment

diptorupd Mar 31, 2021

diptorupd Mar 31, 2021

1e-to Mar 31, 2021

diptorupd Mar 31, 2021

oleksandr-pavlyk Mar 31, 2021

oleksandr-pavlyk Mar 31, 2021

oleksandr-pavlyk Mar 31, 2021

oleksandr-pavlyk Mar 31, 2021

oleksandr-pavlyk Mar 31, 2021

oleksandr-pavlyk commented Apr 2, 2021

Subdevices work #326

Subdevices work #326

Conversation

1e-to commented Mar 17, 2021

1e-to commented Mar 22, 2021

diptorupd commented Mar 22, 2021

1e-to commented Mar 26, 2021 • edited Loading

1e-to commented Mar 29, 2021

oleksandr-pavlyk commented Mar 30, 2021

diptorupd 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleksandr-pavlyk commented Apr 2, 2021

1e-to commented Mar 26, 2021 •

edited

Loading