Skip to content

Conversation

@diptorupd
Copy link
Contributor

@diptorupd diptorupd commented Mar 13, 2021

Contains several improvements to queue creation and management. Filter strings are now properly supported, in-order queues may be created, SYCL async errors are handled, and a whole bunch of related changes to both C API and Python API.

This PR is a subset and a cleaned up version of #310 and supersedes that PR.

Diptorup Deb and others added 15 commits March 13, 2021 15:34
1. error_handler function typedef changed
2. DPCTLDevice_Copy, DPCTLQueue_Copy added
3. DPCTLDeviceMgr_GetContextAndDevicePair exported as well as
   the struct it returns.
SyclQueue(device_selector, props)
SyclQueue(sycl_device, props)
SyclQueue(sycl_context, sycl_device, props)

create SyclQueue from given data. A default asynch error handler is
used. The error handler raises SyclAsynchronousError.
Removed get_num_queues, has_cpu_queues, has_gpu_queues.

The context manager accepts device selector string, device,
or a queue itself.
This way the handler does not use `PyErr_WriteUnraisable`, but uses
`PyErr_PrintEx` instead.
to skip tests that would create queue from a device for which it is
impossible (like accelerator device with driver installed, but hardware absent)
@diptorupd
Copy link
Contributor Author

@oleksandr-pavlyk @PokhodenkoSA I am merging this PR as I and @oleksandr-pavlyk have reviewed the changes.

The new code requires more test cases, but that can be done as a follow up work. I do not want to keep this open and make it even bigger.

@diptorupd diptorupd merged commit 521046c into master Mar 15, 2021
@diptorupd diptorupd deleted the feature/queue_manager_v3 branch March 15, 2021 17:17
@PokhodenkoSA
Copy link
Contributor

PokhodenkoSA commented Mar 24, 2021

@diptorupd please squash commits in PRs next time.

@oleksandr-pavlyk
Copy link
Contributor

@PokhodenkoSA I strongly disfavor the practice of enforcing squashing of commits in the PR. Doing so rewrites history (philosophical objection, granted, but development history is lost, commit messages loose their context and relevance, etc.), makes it difficult to share common commits which are not yet in master between branches, and makes it impossible to precision revert one of the faulty commits in the PR later on.

@PokhodenkoSA
Copy link
Contributor

@oleksandr-pavlyk I would like to recommend to make PRs smaller. It is nightmare to review PRs with hundreds lines changed and without proper description.
I like the idea of well organized commits targeted to particular goals. But usually commits in PRs are small, sometimes repetitive and have ugly commit messages. So I thing it is easier to achieve well organized commits with squashed PRs.

@PokhodenkoSA
Copy link
Contributor

I think forcing merging PRs is not right way too. If keeping original commits is important for some reason it is ok.

@oleksandr-pavlyk oleksandr-pavlyk added this to the 0.7.0 milestone Apr 13, 2021
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.

3 participants