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/queue manager v3 #323

Merged
merged 15 commits into from
Mar 15, 2021
Merged

Feature/queue manager v3 #323

merged 15 commits into from
Mar 15, 2021

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
Collaborator

@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