Support for composite devices by ndgrigorian · Pull Request #1993 · IntelPython/dpctl

ndgrigorian · 2025-02-14T00:01:15Z

This PR proposes supporting the oneAPI composite devices extension in dpctl, which exposes multi-tile GPU devices as root devices for each tile, but also provides access to the composite devices from level zero prior to ZE_FLAT_DEVICE_HIERARCHY=FLAT becoming the default. i.e., with ZE_FLAT_DEVICE_HIERARCHY=COMPOSITE, the composite devices are root devices, but with ZE_FLAT_DEVICE_HIERARCHY=COMBINED, a user gains access to these composite devices through dedicated APIs.

Have you provided a meaningful PR description?
Have you added a test, reproducer or referred to an issue with a reproducer?
Have you tested your changes locally for CPU and GPU devices?
Have you made sure that new changes do not introduce compiler warnings?
Have you checked performance impact of proposed changes?
Have you added documentation for your changes, if necessary?
Have you added your changes to the changelog?
If this PR is a work in progress, are you opening the PR as a draft?

github-actions · 2025-02-14T00:37:26Z

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

github-actions · 2025-02-14T00:44:50Z

Array API standard conformance tests for dpctl= ran successfully.
Passed: 893
Failed: 3
Skipped: 118

coveralls · 2025-02-14T00:59:07Z

coverage: 86.379% (-0.4%) from 86.752%
when pulling c23cbcb on support-composite-devices
into 63f5129 on master.

github-actions · 2025-02-21T21:37:10Z

Array API standard conformance tests for dpctl=0.19.0dev0=py310h93fe807_544 ran successfully.
Passed: 894
Failed: 2
Skipped: 118

github-actions · 2025-03-01T09:51:56Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_25 ran successfully.
Passed: 897
Failed: 0
Skipped: 125

github-actions · 2025-03-05T17:46:39Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_27 ran successfully.
Passed: 896
Failed: 1
Skipped: 126

github-actions · 2025-03-06T00:45:04Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_31 ran successfully.
Passed: 895
Failed: 2
Skipped: 126

github-actions · 2025-03-06T03:13:36Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_31 ran successfully.
Passed: 895
Failed: 2
Skipped: 126

github-actions · 2025-03-06T05:56:08Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_34 ran successfully.
Passed: 895
Failed: 2
Skipped: 126

github-actions · 2025-03-06T06:23:52Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_34 ran successfully.
Passed: 895
Failed: 2
Skipped: 126

github-actions · 2025-03-06T06:26:14Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_34 ran successfully.
Passed: 896
Failed: 1
Skipped: 126

github-actions · 2025-03-28T03:45:22Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_82 ran successfully.
Passed: 894
Failed: 2
Skipped: 154

This leverages oneAPI extension for composite devices to add the free function `ext_oneapi_get_composite_devices` to the main dpctl namespace

This method is only applicable for level_zero backend, returning an empty list for all other backend types

…aspect_component` Aligns with the rest of the SyclDevice properties Adds tests for the aspects

Rename some variables for consistency

github-actions · 2025-03-31T17:20:09Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_85 ran successfully.
Passed: 894
Failed: 2
Skipped: 154

ndgrigorian · 2025-04-01T01:48:45Z

@antonwolfy
ping

dpctl/__init__.py

dpctl/_sycl_device.pyx

dpctl/_sycl_device_factory.pyx

dpctl/_sycl_platform.pyx

dpctl/tests/test_sycl_device.py

libsyclinterface/tests/test_sycl_device_aspects.cpp

dpctl/_sycl_device.pyx

libsyclinterface/source/dpctl_sycl_device_interface.cpp

…vices_from_composite

github-actions · 2025-04-01T17:54:15Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_99 ran successfully.
Passed: 894
Failed: 2
Skipped: 154

antonwolfy

Thank you @ndgrigorian, LGTM!

github-actions · 2025-04-01T18:38:02Z

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_100 ran successfully.
Passed: 894
Failed: 2
Skipped: 154

AlexanderKalistratov · 2025-04-02T12:39:58Z

libsyclinterface/source/dpctl_sycl_device_interface.cpp

+DPCTLDevice_GetComponentDevices(__dpctl_keep const DPCTLSyclDeviceRef DRef)
+{
+    using vecTy = std::vector<DPCTLSyclDeviceRef>;
+    vecTy *ComponentDevicesVectorPtr = nullptr;


You can use unique_ptr here to avoid calling delete by hands.

__dpctl_give DPCTLDeviceVectorRef DPCTLDevice_GetComponentDevices(__dpctl_keep const DPCTLSyclDeviceRef DRef) { using vecTy = std::vector<DPCTLSyclDeviceRef>; if (!DRef) { return nullptr; } auto D = unwrap<device>(DRef); try { auto componentDevices = D->get_info<sycl::ext::oneapi::experimental::info::device:: component_devices>(); auto ComponentDevicesVectorPtr = std::make_unique(new vecTy()); ComponentDevicesVectorPtr->reserve(componentDevices.size()); for (const auto &cd : componentDevices) { ComponentDevicesVectorPtr->emplace_back( wrap<device>(new device(cd))); } return wrap<vecTy>(ComponentDevicesVectorPtr.release()); } catch (std::exception const &e) { error_handler(e, __FILE__, __func__, __LINE__); } return nullptr; }

This is something that might be smart to go back and do throughout the libsyclinterface codebase

AlexanderKalistratov · 2025-04-02T12:47:34Z

libsyclinterface/source/dpctl_sycl_device_interface.cpp

+__dpctl_give DPCTLSyclDeviceRef
+DPCTLDevice_GetCompositeDevice(__dpctl_keep const DPCTLSyclDeviceRef DRef)
+{
+    auto D = unwrap<device>(DRef);


This function feels a bit overcomplicated. Is this done for purpose?

__dpctl_give DPCTLSyclDeviceRef DPCTLDevice_GetCompositeDevice(__dpctl_keep const DPCTLSyclDeviceRef DRef) { if (!DRef) { return nullptr; } auto D = unwrap<device>(DRef); try { bool is_component = D->has(sycl::aspect::ext_oneapi_is_component); if (is_component) { const auto &compositeDevice = D->get_info<sycl::ext::oneapi::experimental::info::device:: composite_device>(); return wrap<device>(new device(compositeDevice)); } } catch (std::exception const &e) { error_handler(e, __FILE__, __func__, __LINE__); } return nullptr; }

AlexanderKalistratov · 2025-04-02T12:55:55Z

libsyclinterface/source/dpctl_sycl_device_manager.cpp

+ */
+__dpctl_give DPCTLDeviceVectorRef DPCTLDeviceMgr_GetCompositeDevices()
+{
+    using vecTy = std::vector<DPCTLSyclDeviceRef>;


Same here. unique_ptr + combine two try...catch into one:

__dpctl_give DPCTLDeviceVectorRef DPCTLDeviceMgr_GetCompositeDevices() { using vecTy = std::vector<DPCTLSyclDeviceRef>; try { auto Devices = std::make_unique(new vecTy()); auto composite_devices = ext::oneapi::experimental::get_composite_devices(); Devices->reserve(composite_devices.size()); for (const auto &CDev : composite_devices) { Devices->emplace_back(wrap<device>(new device(std::move(CDev)))); } return wrap<vecTy>(Devices.release()); } catch (std::exception const &e) { error_handler(e, __FILE__, __func__, __LINE__); } return nullptr; }

AlexanderKalistratov · 2025-04-02T13:18:22Z

libsyclinterface/source/dpctl_sycl_platform_interface.cpp

 }
+
+__dpctl_give DPCTLDeviceVectorRef
+DPCTLPlatform_GetCompositeDevices(__dpctl_keep const DPCTLSyclPlatformRef PRef)


Same here:

__dpctl_give DPCTLDeviceVectorRef DPCTLPlatform_GetCompositeDevices(__dpctl_keep const DPCTLSyclPlatformRef PRef) { if (!PRef) { error_handler("Cannot retrieve composite devices from " "DPCTLSyclPlatformRef as input is a nullptr.", __FILE__, __func__, __LINE__); return nullptr; } auto P = unwrap<platform>(PRef); using vecTy = std::vector<DPCTLSyclDeviceRef>; try { auto DevicesVectorPtr = std::unique_ptr<>(new vecTy()); auto composite_devices = P->ext_oneapi_get_composite_devices(); DevicesVectorPtr->reserve(composite_devices.size()); for (const auto &Dev : composite_devices) { DevicesVectorPtr->emplace_back( wrap<device>(new device(std::move(Dev)))); } return wrap<vecTy>(DevicesVectorPtr.release()); } catch (std::exception const &e) { error_handler(e, __FILE__, __func__, __LINE__); } return nullptr; }

AlexanderKalistratov · 2025-04-02T13:42:28Z

libsyclinterface/source/dpctl_sycl_device_interface.cpp

+{
+    using vecTy = std::vector<DPCTLSyclDeviceRef>;
+    vecTy *ComponentDevicesVectorPtr = nullptr;
+    if (DRef) {


In some functions we are checking for XRef to be non nullptr. (https://github.com/IntelPython/dpctl/pull/1993/files#diff-93f8a41da82dd5dd2ae6b05e019d8104c87c244e6077d5a819625a2fc5e3a2f1R858)

In others we are checking that result of auto X = unwrap<...>(XRef) is non nullptr. (https://github.com/IntelPython/dpctl/pull/1993/files#diff-93f8a41da82dd5dd2ae6b05e019d8104c87c244e6077d5a819625a2fc5e3a2f1R883, https://github.com/IntelPython/dpctl/pull/1993/files#diff-2cee49fe41f858d191076f530e6c6ce6ca841be844b8bca525509ed3deddbbaeR324)

Is that intentionally?
Are these checks interchangeable?
Can we unwrap<...>(XRef) if XRef is nullptr?
Can result of unwrap<...>(XRef) be nullptr if XRef is not nullptr?

They are treated as more or less interchangeable throughout the file, and throughout libsyclinterface.

As to if we can unwrap a nullptr, yes, there are a few tests which more or less do this throughout the libsyclinterface tests.

And the last question, I can't think of such a case. Given it's treated as interchangeable: probably not.

AlexanderKalistratov

A bit late, but I have some comments:

try to avoid calling delete by hands and use unique_ptr instead
In most of the functions two try...catch blocks could be combined into one.
I have question about equivalence of two checks:

if (XRef) {
    auto X = unwrap<...>(XRef);
    ...
}

vs

auto X = unwrap<...>(XRef);
if (X) {
    ...
}

ndgrigorian · 2025-04-02T17:33:53Z

A bit late, but I have some comments:

try to avoid calling delete by hands and use unique_ptr instead

In most of the functions two try...catch blocks could be combined into one.

I have question about equivalence of two checks:
if (XRef) {
    auto X = unwrap<...>(XRef);
    ...
}
vs
auto X = unwrap<...>(XRef);
if (X) {
    ...
}

Suggestions themselves make sense, I think it may be most sensible to open up (a) PR(s) doing some refactoring and clean-up in libsyclinterface

ndgrigorian force-pushed the support-composite-devices branch 2 times, most recently from 2d5942d to 76f2fc2 Compare February 21, 2025 20:55

ndgrigorian force-pushed the support-composite-devices branch from 76f2fc2 to 00cb089 Compare March 1, 2025 09:10

ndgrigorian force-pushed the support-composite-devices branch from 00cb089 to cc5ef08 Compare March 5, 2025 17:03

ndgrigorian force-pushed the support-composite-devices branch from 7ea744b to b8c765f Compare March 6, 2025 02:31

ndgrigorian force-pushed the support-composite-devices branch 2 times, most recently from 4ab4cb0 to 8327887 Compare March 6, 2025 05:44

ndgrigorian marked this pull request as ready for review March 6, 2025 05:45

ndgrigorian requested review from antonwolfy, vlad-perevezentsev and vtavana as code owners March 6, 2025 05:45

ndgrigorian force-pushed the support-composite-devices branch from 8327887 to d27f1d0 Compare March 28, 2025 03:02

ndgrigorian added 6 commits March 31, 2025 09:34

Add dpctl.get_composite_devices

f787027

This leverages oneAPI extension for composite devices to add the free function `ext_oneapi_get_composite_devices` to the main dpctl namespace

Adds get_composite_devices method to dpctl.SyclPlatform

92bcdfd

This method is only applicable for level_zero backend, returning an empty list for all other backend types

Add composite device related attributes to dpctl.SyclDevice

d431ca1

Add a test for get_composite_devices

752d927

is_composite and is_component -> has_aspect_composite and `has_…

20ab076

…aspect_component` Aligns with the rest of the SyclDevice properties Adds tests for the aspects

Adds is_component and is_composite as dpctl aspect strings

a8aff3d

ndgrigorian added 4 commits March 31, 2025 09:34

Adds C-API tests for composite devices

c2274aa

More C-API tests for composite devices

cc1e023

Rename some variables for consistency

Add a test for component and composite device methods

be613bb

Add tests for dpctl.SyclPlatform.get_composite_devices

9cb7b50

ndgrigorian force-pushed the support-composite-devices branch from d27f1d0 to 9cb7b50 Compare March 31, 2025 16:34

ndgrigorian requested a review from AlexanderKalistratov April 1, 2025 02:00

antonwolfy reviewed Apr 1, 2025

View reviewed changes

ndgrigorian added 7 commits April 1, 2025 08:40

Add dpctl.get_composite_devices to docs

1013422

Remove debug print in SyclDevice aspect tests

ea4101f

Fix mistake in dpctl.SyclPlatform.get_composite_devices docstring

cf6fce2

Fix dpctl.SyclDevice.composite_device docstring

d51657a

Check component and composite device aspects in test_get_component_de…

1821b76

…vices_from_composite

Use reserve in DPCTLDevice_GetComponentDevices

340ffa2

SyclSubdeviceCreationError -> SyclSubDeviceCreationError

c23cbcb

antonwolfy approved these changes Apr 1, 2025

View reviewed changes

ndgrigorian merged commit 67317b0 into master Apr 1, 2025
71 of 72 checks passed

ndgrigorian deleted the support-composite-devices branch April 1, 2025 20:49

AlexanderKalistratov reviewed Apr 2, 2025

View reviewed changes

Conversation

ndgrigorian commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 14, 2025

Uh oh!

coveralls commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 21, 2025

Uh oh!

github-actions bot commented Mar 1, 2025

Uh oh!

github-actions bot commented Mar 5, 2025

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

github-actions bot commented Mar 28, 2025

Uh oh!

github-actions bot commented Mar 31, 2025

Uh oh!

ndgrigorian commented Apr 1, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Apr 1, 2025

Uh oh!

antonwolfy left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 1, 2025

Uh oh!

Uh oh!

AlexanderKalistratov Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ndgrigorian Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

AlexanderKalistratov Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderKalistratov Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderKalistratov Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

AlexanderKalistratov Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

ndgrigorian Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderKalistratov left a comment

Choose a reason for hiding this comment

Uh oh!

ndgrigorian commented Apr 2, 2025

ndgrigorian commented Feb 14, 2025 •

edited

Loading

github-actions bot commented Feb 14, 2025 •

edited

Loading

coveralls commented Feb 14, 2025 •

edited

Loading

AlexanderKalistratov Apr 2, 2025 •

edited

Loading

AlexanderKalistratov Apr 2, 2025 •

edited

Loading

AlexanderKalistratov Apr 2, 2025 •

edited

Loading

ndgrigorian Apr 2, 2025 •

edited

Loading