change and fixes for existing funcs #6

1e-to · 2020-10-12T16:11:06Z

For now in this PR I did following:

Fix a typo with the name of one of the getter DPPLDevice_GetMaxComputeUnites to DPPLDevice_GetMaxComputeUnits
Add the function declarations for the getters to backend.pxd.
Add dppl_utils.h/DPPLSize_t_Array_Delete to backend.pxd.
Add members and getters for all the attributes to SyclDevice class in sycl_core.pyx and _sycl_core.pxd.
Also change the existing getters get_device_name, get_device_type, get_vendor_name, get_driver_version also to cpdef.
Add unit tests for the Cython functions to a new test_sycl_device.py file.

WIP

-- default device is always available.

Solution is not to create context on devices that are not of the requested device type or not from the requested backend. ``` Python 3.7.7 (default, Jul 14 2020, 22:02:37) Type 'copyright', 'credits' or 'license' for more information IPython 7.17.0 -- An enhanced Interactive Python. Type '?' for help. In [1]: import dpctl In [2]: with dpctl.device_context('opencl:gpu'): ...: print(1) ...: 1 In [3]: with dpctl.device_context('level0:gpu'): ...: print(1) ...: 1 ```

…pecifier.

…rupd/dpctl into feature/backend_specific_queues_v2

* CC=clang.exe * CC=clang-cl.exe

…ific_queues_v2 Feature/backend specific queues v2

…sue_83

* Add support for uint8 and ulong.

…aster

* Rename all functions to addressof_ref and add docstring. Co-authored-by: reazul.hoque <[email protected]>

* Rename readme and headers * Fix left over places where DPPL/PyDPPL was used in comments. Co-authored-by: Diptorup Deb <[email protected]>

* Fix unhandled character * Fix unhandled character * Remove unused imports * Fix class name duplication

Warning about conda-build=3.20

* Add changelog Co-authored-by: etotmeni <[email protected]> Co-authored-by: Diptorup Deb <[email protected]>

This ensures that backend libraries are copied into dpctl and included in the conda tar ball. Also fixed copy statement to account for .lib files being in %INSTALL_PREFIX%\lib and .dll files being in %INSTALL_PREFIX%\bin on Windows.

Use INSTALL_PATH in cmake call

* Fixed typo in the code for set_default_queue Allowing both strings or enums as selectors for backend and device. * Fixed "variabel reference before assignment" in device_context manager ctxt must be set outside of try/finally to be guaranteed avaialble in the finally clause.

diptorupd · 2020-10-13T21:55:18Z

dpctl/sycl_core.pyx

+        max_work_item_sizes = []
+        for n in range(3):
+            max_work_item_sizes.append(self._max_work_item_sizes[n])
+        DPPLSize_t_Array_Delete(self._max_work_item_sizes)


Should the DPPLSize_t_Array_Delete call be here? I think should be moved to the __dealloc__ otherwise the self._max_work_item_sizes is deleted after first call to get_max_work_item_sizes.

diptorupd · 2020-10-13T22:04:38Z

dpctl/tests/test_sycl_device.py

+
+    def test_get_max_compute_units (self):
+        q = dpctl.get_current_queue()
+        try:


Also test if max_compute_units is returned some expected value and not garbage?

diptorupd · 2020-10-13T22:05:53Z

dpctl/tests/test_sycl_device.py

+class TestSyclDevice (unittest.TestCase):
+
+    def test_get_max_compute_units (self):
+        q = dpctl.get_current_queue()


Maybe check for exception on get_current_queue as well since that call can raise a SyclQueueCreationError.

diptorupd · 2020-10-13T22:06:07Z

dpctl/tests/test_sycl_device.py

+    def test_get_max_work_item_dims (self):
+        q = dpctl.get_current_queue()
+        try:
+            max_work_item_dims = q.get_sycl_device().get_max_work_item_dims()


diptorupd · 2020-10-13T22:06:17Z

dpctl/tests/test_sycl_device.py

+    def test_get_max_work_item_sizes (self):
+        q = dpctl.get_current_queue()
+        try:
+            max_work_item_sizes = q.get_sycl_device().get_max_work_item_sizes()


diptorupd · 2020-10-13T22:06:31Z

dpctl/tests/test_sycl_device.py

+    def test_get_max_work_group_size (self):
+        q = dpctl.get_current_queue()
+        try:
+            max_work_group_size = q.get_sycl_device().get_max_work_group_size()


diptorupd · 2020-10-13T22:06:39Z

dpctl/tests/test_sycl_device.py

+    def test_get_max_num_sub_groups (self):
+        q = dpctl.get_current_queue()
+        try:
+            max_num_sub_groups = q.get_sycl_device().get_max_num_sub_groups()


diptorupd · 2020-10-13T22:08:20Z

backends/include/dppl_sycl_device_interface.h

+ */
+DPPL_API
+bool
+DPPLDevice_GetAspectsBaseAtomics (__dppl_keep const DPPLSyclDeviceRef DRef);


Let us rename to DPPLDevice_HasInt64BaseAtomics.

diptorupd · 2020-10-13T22:08:59Z

backends/include/dppl_sycl_device_interface.h

+ */
+DPPL_API
+bool
+DPPLDevice_GetAspectsExtendedAtomics (__dppl_keep const DPPLSyclDeviceRef DRef);


let us rename to DPPLDevice_HasInt64ExtendedAtomics

diptorupd · 2020-10-13T22:09:48Z

backends/source/dppl_sycl_device_interface.cpp

+{
+    auto D = unwrap(DRef);
+    if(D) {
+        return D->has(aspect::int64_base_atomics);


I did not know about the functions. Nice!

diptorupd · 2020-10-13T22:14:47Z

backends/tests/test_sycl_device_interface.cpp

+    if(!OpenCL_cpu)
+        GTEST_SKIP_("Skipping as no OpenCL CPU device found.");
+
+    auto n = DPPLDevice_GetAspectsBaseAtomics(OpenCL_cpu);


What do we expect on devices that actually do not have int64_base_atomics? In such cases false is the right result. I think a better way to test this is to include CL::sycl.hpp and call the get the aspect for the device using SYCL and see if it matches what DPPLDevice_xxx returns. It is not really a great test, just sanity check.

diptorupd · 2020-10-13T22:15:01Z

backends/tests/test_sycl_device_interface.cpp

+
+    auto n = DPPLDevice_GetAspectsBaseAtomics(OpenCL_gpu);
+    EXPECT_TRUE(n != false);
+}


diptorupd · 2020-10-13T22:15:20Z

backends/tests/test_sycl_device_interface.cpp

+        GTEST_SKIP_("Skipping as no OpenCL CPU device found.");
+
+    auto n = DPPLDevice_GetAspectsExtendedAtomics(OpenCL_cpu);
+    EXPECT_TRUE(n != false);


diptorupd · 2020-10-13T22:15:28Z

backends/tests/test_sycl_device_interface.cpp

+        GTEST_SKIP_("Skipping as no OpenCL GPU device found.");
+
+    auto n = DPPLDevice_GetAspectsExtendedAtomics(OpenCL_gpu);
+    EXPECT_TRUE(n != false);


diptorupd · 2020-10-13T22:17:22Z

dpctl/tests/test_sycl_device.py


+    def test_get_aspects_base_atomics (self):
+        q = dpctl.get_current_queue()
+        try:


Aha now I see. Basically we are testing that the function should never raise an exception. OK.

See https://github.com/psf/black#show-your-style

This GitHub Action checks that code in pull request is formatted with black. See https://black.readthedocs.io/en/stable/github_actions.html

versioneer.py is not a part of the project. Add pyproject.toml file with section [tool.black] and parameter exclude = 'versioneer.py'. This file will be used by developers and GitHub Actions for black. Usign pyproject.toml allows use black like this: `black .`

Use black version 20.8b1 or branch [stable](https://github.com/psf/black/tree/stable)). Command: black .

File .git-blame-ignore-revs contains a list of revisions to use for ignoring in blame. Example: $ git blame FILE --ignore-revs-file .git-blame-ignore-revs Also you can configure git to automatically ignore revisions listed in a file on every call to git blame: $ git config blame.ignoreRevsFile .git-blame-ignore-revs Requirements: Git >=2.23

Add black formatter for Python code

…t-queue-on-C-side dpctl.get_current_queue is now cimportable

PokhodenkoSA · 2020-10-19T14:12:16Z

backends/include/dppl_sycl_device_interface.h

 DPPL_API
 uint32_t
-DPPLDevice_GetMaxComputeUnites (__dppl_keep const DPPLSyclDeviceRef DRef);
+DPPLDevice_GetMaxComputeUnits (__dppl_keep const DPPLSyclDeviceRef DRef);


It is very dangerous to rename API function. It is better to create a new one with new name and implement the old one via the new one.

Then mark it as deprecated. Better to print warning in stderr when old function is used.
And remove in the next release.

See https://wiki.debian.org/HardeningWalkthrough#Testing_your_packages_after_conversion

* Del old ocl interface * Del cffi * Fix CMakeLists * Remove OpenCL from README.md * Update changelog Co-authored-by: etotmeni <[email protected]> Co-authored-by: Sergey Pokhodenko <[email protected]>

Diptorup Deb and others added 30 commits October 4, 2020 09:17

Add support for uint8 and ulong.

1de7ff4

A current device is always present even outside a device context.

ccb8d4c

-- default device is always available.

Allow program creation only for OpenCL for the time being.

3f92407

Fix errors found for Sergey.

530ae9f

Remove useless except+, as C++ exceptions do not cross C API.

d6bcd74

no need to use enum in return type

4e6aabc

Rename DPPLSyclBEType to DPPLSyclBackendType. Drop superfluous enum s…

772b97d

…pecifier.

Merge branch 'feature/backend_specific_queues_v2' of github.com:dipto…

c4acc54

…rupd/dpctl into feature/backend_specific_queues_v2

Reformat.

5d6ddc4

Fix formatting.

81dd13d

onaAPI beta10 compiler (IntelPython#103)

071a5fe

* CC=clang.exe * CC=clang-cl.exe

onaAPI beta10 compiler (IntelPython#103)

7586726

* CC=clang.exe * CC=clang-cl.exe

Merge pull request IntelPython#93 from diptorupd/feature/backend_spec…

251f135

…ific_queues_v2 Feature/backend specific queues v2

Add support for uint8 and ulong.

35c4748

Merge branch 'fix/issue_83' of github.com:diptorupd/dpctl into fix/is…

1e0db44

…sue_83

Merge branch 'diptorupd-fix/issue_83' into upstream_master

f765236

Fix/issue 83 (IntelPython#96)

7f7b090

* Add support for uint8 and ulong.

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

d10f0e3

…aster

Feature/get ref address (IntelPython#105)

451ca50

* Rename all functions to addressof_ref and add docstring. Co-authored-by: reazul.hoque <[email protected]>

Rename PyDPPL to dpCtl in all visible places (IntelPython#104)

bb41bb6

* Rename readme and headers * Fix left over places where DPPL/PyDPPL was used in comments. Co-authored-by: Diptorup Deb <[email protected]>

Fix bugs highlighted by tools (IntelPython#92)

aaa51d3

* Fix unhandled character * Fix unhandled character * Remove unused imports * Fix class name duplication

Update README.md

e351cb2

Warning about conda-build=3.20

Change cast from long to size_t (IntelPython#111)

9449986

Add changelog (IntelPython#108)

9e3d086

* Add changelog Co-authored-by: etotmeni <[email protected]> Co-authored-by: Diptorup Deb <[email protected]>

Update CONTRIBUTING.md

438d620

Use INSTALL_PATH in cmake call

59fab30

This ensures that backend libraries are copied into dpctl and included in the conda tar ball. Also fixed copy statement to account for .lib files being in %INSTALL_PREFIX%\lib and .dll files being in %INSTALL_PREFIX%\bin on Windows.

Merge pull request IntelPython#114 from IntelPython/fix-windows-script

38d9993

Use INSTALL_PATH in cmake call

Update CHANGELOG.md

49042af

diptorupd reviewed Oct 13, 2020

View reviewed changes

Fixes according comments

e3c901d

1e-to force-pushed the extra_device_info branch from 75a229a to e3c901d Compare October 14, 2020 16:15

etotmeni and others added 9 commits October 14, 2020 12:02

Fix tests

19abe76

Use the black badge in project's README.md

d114c8c

See https://github.com/psf/black#show-your-style

Add GitHub Actions for black

eb8afec

This GitHub Action checks that code in pull request is formatted with black. See https://black.readthedocs.io/en/stable/github_actions.html

Format all Python code with black

41ccd65

Use black version 20.8b1 or branch [stable](https://github.com/psf/black/tree/stable)). Command: black .

Update CONTRIBUTING.md

c9ab60a

Merge pull request IntelPython#91 from PokhodenkoSA/tools/black

519312f

Add black formatter for Python code

Merge pull request IntelPython#125 from IntelPython/expose-get-curren…

6684c7b

…t-queue-on-C-side dpctl.get_current_queue is now cimportable

PokhodenkoSA reviewed Oct 19, 2020

View reviewed changes

etotmeni and others added 14 commits October 19, 2020 10:09

merge master

7a4750a

Codestyle

e509c23

Codestyle

245cafc

Some fixes + add tests for level0 gpu

1c151b9

Merge branch 'upstream_master'

425d841

Change names in tests level0gpu

d19a63d

Add flags for fixing hardening-check (IntelPython#149)

519b077

See https://wiki.debian.org/HardeningWalkthrough#Testing_your_packages_after_conversion

Remove OpenCL interface (IntelPython#138)

d8e7104

* Del old ocl interface * Del cffi * Fix CMakeLists * Remove OpenCL from README.md * Update changelog Co-authored-by: etotmeni <[email protected]> Co-authored-by: Sergey Pokhodenko <[email protected]>

Merge branch 'upstream_master'

8c4ffbb

merge master

7e5ceb3

Update changelog

6bd898d

fixed filename typo in setup.py to fix build

7d548cd

Merge branch 'master' into extra_device_info

a08c943

Fix import _backend in _sycl_core.pxd

c26f332

1e-to closed this Oct 29, 2020

1e-to deleted the extra_device_info branch March 2, 2021 12:13

change and fixes for existing funcs #6

change and fixes for existing funcs #6

Uh oh!

Conversation

1e-to commented Oct 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diptorupd Oct 13, 2020 • 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!

diptorupd Oct 13, 2020 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

diptorupd Oct 13, 2020 •

edited

Loading

diptorupd Oct 13, 2020 •

edited

Loading