Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/sycl context #334

Merged
merged 22 commits into from
Mar 31, 2021
Merged

Feature/sycl context #334

merged 22 commits into from
Mar 31, 2021

Conversation

oleksandr-pavlyk
Copy link
Collaborator

@diptorupd This is a WIP to add SyclContext constructors, including those multi-device context.

In [1]: import dpctl

In [2]: dpctl.SyclContext() # default device context
Out[2]: <dpctl._sycl_context.SyclContext at 0x7f987c330770>

In [3]: Out[2].get_devices()
Out[3]: [<dpctl.SyclDevice [backend_type.level_zero, device_type.gpu,  Intel(R) Graphics [0x9bca]] at 0x7f986c81b0b0>]

In [4]: dpctl.SyclContext("opencl:cpu")
Out[4]: <dpctl._sycl_context.SyclContext at 0x7f986c84e630>

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

This works

In [6]: d = dpctl.SyclDevice("opencl:cpu")

In [7]: ctx = dpctl.SyclContext(d)

In [9]: q = dpctl.SyclQueue(ctx, d)

In [10]: q
Out[10]: <dpctl.SyclQueue at 0x7f985b58ac30>

In [11]: ctx
Out[11]: <dpctl._sycl_context.SyclContext at 0x7f986c84e330>

In [12]: q.get_sycl_context()
Out[12]: <dpctl._sycl_context.SyclContext at 0x7f986c84e330>

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

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

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

However, when using different instances of device, it does not work:

In [16]: dpctl.SyclQueue( dpctl.SyclContext(dpctl.select_cpu_device()), dpctl.select_cpu_device())
Queue cannot be constructed with the given context and device as the context does not contain the given device. -33 (CL_INVALID_DEVICE)
---------------------------------------------------------------------------
SyclQueueCreationError                    Traceback (most recent call last)
<ipython-input-16-9313ae43729f> in <module>()
----> 1 dpctl.SyclQueue( dpctl.SyclContext(dpctl.select_cpu_device()), dpctl.select_cpu_device())

~/work/dpctl/dpctl/_sycl_queue.pyx in dpctl._sycl_queue.SyclQueue.__cinit__()
    230                 if len_args == 2:
    231                     arg = args
--> 232                 raise SyclQueueCreationError(
    233                     "SYCL Queue failed to be created from '{}'.".format(arg)
    234                 )

SyclQueueCreationError: SYCL Queue failed to be created from '(<dpctl._sycl_context.SyclContext object at 0x7f986c84ee10>, <dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7f9848071eb0>)'.

@oleksandr-pavlyk oleksandr-pavlyk marked this pull request as draft March 25, 2021 16:11
@oleksandr-pavlyk
Copy link
Collaborator Author

Minimal example:

In [4]: d = dpctl.select_cpu_device()

In [5]: ctx_ok =  dpctl.SyclContext(d)

In [6]: ctx_bad = dpctl.SyclContext(dpctl.select_cpu_device())

In [7]: dpctl.SyclQueue(ctx_ok, d)
Out[7]: <dpctl.SyclQueue at 0x7fbf4c2f50a0>

In [8]: dpctl.SyclQueue(ctx_bad, d)
Queue cannot be constructed with the given context and device as the context does not contain the given device. -33 (CL_INVALID_DEVICE)
---------------------------------------------------------------------------
SyclQueueCreationError                    Traceback (most recent call last)
<ipython-input-8-976f72760c0f> in <module>()
----> 1 dpctl.SyclQueue(ctx_bad, d)

/repos/dpctl/dpctl/_sycl_queue.pyx in dpctl._sycl_queue.SyclQueue.__cinit__()
    230                 if len_args == 2:
    231                     arg = args
--> 232                 raise SyclQueueCreationError(
    233                     "SYCL Queue failed to be created from '{}'.".format(arg)
    234                 )

SyclQueueCreationError: SYCL Queue failed to be created from '(<dpctl._sycl_context.SyclContext object at 0x7fbf8c04fc90>, <dpctl.SyclDevice [backend_type.opencl, device_type.cpu,  Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz] at 0x7fbf876d7cb0>)'.

dpctl/_sycl_context.pyx Outdated Show resolved Hide resolved
dpctl/_sycl_context.pyx Outdated Show resolved Hide resolved
@diptorupd
Copy link
Contributor

@oleksandr-pavlyk Please rebase on top of #338.

@oleksandr-pavlyk
Copy link
Collaborator Author

@oleksandr-pavlyk Please rebase on top of #338.

I think it is better for #338 to get merged into master, and for me to merge master into this branch. If the intent is to experiment with #338, then I can always merge the associated branch into this one locally to test.

Have 2021 update 2 been released yet?

@oleksandr-pavlyk oleksandr-pavlyk marked this pull request as ready for review March 29, 2021 16:40
@oleksandr-pavlyk
Copy link
Collaborator Author

@oleksandr-pavlyk Please rebase on top of #338.

I think it is better for #338 to get merged into master, and for me to merge master into this branch. If the intent is to experiment with #338, then I can always merge the associated branch into this one locally to test.

Have 2021 update 2 been released yet?

Merged #338 here.

@oleksandr-pavlyk
Copy link
Collaborator Author

I think this PR is ready. SyclContext constructor is in place, tests are in place.

Linux tests passed. Windows - pytest run seg-faults:

  tests/test_dparray.py::Test_dparray::test_dparray_T <- ..\_test_env\lib\site-packages\dpctl\tests\test_dparray.py
  tests.test_dparray.Test_dparray.test_dparray_T
  Fatal Python error: Aborted

Copy link
Contributor

@diptorupd diptorupd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oleksandr-pavlyk Please move the changes to sycl_queue.pyx and sycl_platform.pyx and related pxd files into a separate PR. Apart from that some minor changes that I found.

dpctl-capi/source/dpctl_sycl_context_interface.cpp Outdated Show resolved Hide resolved
dpctl-capi/source/dpctl_vector_templ.cpp Show resolved Hide resolved
dpctl-capi/source/dpctl_vector_templ.cpp Outdated Show resolved Hide resolved
dpctl-capi/tests/test_sycl_context_interface.cpp Outdated Show resolved Hide resolved
dpctl/_sycl_context.pyx Outdated Show resolved Hide resolved
dpctl/_sycl_context.pyx Outdated Show resolved Hide resolved
dpctl/_sycl_context.pyx Outdated Show resolved Hide resolved
dpctl/_sycl_context.pyx Outdated Show resolved Hide resolved
dpctl/_sycl_device.pyx Outdated Show resolved Hide resolved
@oleksandr-pavlyk oleksandr-pavlyk force-pushed the feature/SyclContext branch 4 times, most recently from fb1bf0c to 2ad7a16 Compare March 31, 2021 20:58
@oleksandr-pavlyk
Copy link
Collaborator Author

@oleksandr-pavlyk Please move the changes to sycl_queue.pyx and sycl_platform.pyx and related pxd files into a separate PR.

Because of several merging that resolved conflicts, it is an involved operation. I will attempt it, but frankly I see little value in doing this exercise.

@diptorupd
Copy link
Contributor

@oleksandr-pavlyk Please move the changes to sycl_queue.pyx and sycl_platform.pyx and related pxd files into a separate PR.

Because of several merging that resolved conflicts, it is an involved operation. I will attempt it, but frankly I see little value in doing this exercise.

Just a suggestion. Let us not bother if it is too involved.

Added test to cover that functionality, added it to _backend.pyx
[X] Implementation
[X] Header + Documentation comment
[X] Two tests in test_sycl_context.cpp
[X] Exported in `dpctl/_backend.pxd`
Returns list of SyclDevice objects associated with the given queue.
Added method device_count (perhaps could be a property), and used it to
give different __repr__ outputs for single-device contexts and multi-device
context instances.
When creating from a device, we must look up cached DeviceAndContext
pair first, and only if that fails call DPCTLContext_Create

SyclContext was not doing a look-up, and SyclQueue was not doing the
creation is the lookup were to fail.
Added two more tests to test_sycl_context.py

One of them is skipped due to a bug in DPC++, which is expected to be
fixed in update 2 of oneAPI 2021
Also fixed alignment of colons in the docstring.
Prominently state that _create deletes its argument reference variable.

DPCTL opaque pointers will be consumed by most _init_* functions.

Enabled test which was xfailed waiting for the DPCPP update.
references by opaque pointers in the given array, making these copies
is removed from Cython.
Copy link
Contributor

@diptorupd diptorupd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Sasha!

@diptorupd diptorupd merged commit ec0de51 into master Mar 31, 2021
@diptorupd diptorupd deleted the feature/SyclContext branch March 31, 2021 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants