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

option nogil=True not required for ufuncs and gufuncs #6445

Closed
wants to merge 2 commits into from

Conversation

pradkrish
Copy link

@pradkrish pradkrish commented Nov 2, 2020

Closes #1317
nogil=True option is not required for ufuncs and gufuncs. Print a UserWarning if it is provided.

@sklam sklam added this to the Numba 0.53 RC milestone Nov 2, 2020
Copy link
Member

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

CUDA ufuncs don't release the GIL, for various reasons (some Python code needed around to setup arguments, kernel launch asynchrony, etc.). When trying to use nogil=True an error is produced. For example:

@vectorize(nogil=True, target='cuda')
def f(x, y):
    return x + y

Gives:

KeyError: "cuda vectorize target does not support option: 'nogil'"

This is all fine with the functionality in this PR: However, I think the message in the warning is a little misleading - perhaps "ufuncs and gufuncs always release the GIL with the CPU target" would be more accurate?

@pradkrish
Copy link
Author

This is all fine with the functionality in this PR: However, I think the message in the warning is a little misleading - perhaps "ufuncs and gufuncs always release the GIL with the CPU target" would be more accurate?

Thanks for the review, I have modified the warning.

Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, just a couple of minor things to resolve else good to go! Thanks again!

Comment on lines +87 to +88
warnings.warn("nogil option is not required, ufuncs and gufuncs "
"always releases GIL for CPU target")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest using numba.errors.NumbaWarning as the warning category such that it's easier for users to filter out things from Numba.

Comment on lines +87 to +88
warnings.warn("nogil option is not required, ufuncs and gufuncs "
"always releases GIL for CPU target")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
warnings.warn("nogil option is not required, ufuncs and gufuncs "
"always releases GIL for CPU target")
warnings.warn("The 'nogil' option is not required, ufuncs and gufuncs "
"always release the GIL on the CPU target")

Comment on lines +208 to +209
def test_warning_vectorize_with_nogil(self):
@vectorize(nogil=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to check guvectorize as well given the error message mentions it?

@stuartarchibald stuartarchibald added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Nov 6, 2020
@sklam sklam modified the milestones: PR Backlog, Numba 0.54 RC Jan 21, 2021
@esc
Copy link
Member

esc commented Jan 21, 2021

@pradkrish just enquiring about the state of this PR, where is it at?

@github-actions
Copy link

This pull request is marked as stale as it has had no activity in the past 3 months. Please respond to this comment if you're still interested in working on this. Many thanks!

@github-actions github-actions bot added the stale Marker label for stale issues. label Mar 19, 2023
@github-actions github-actions bot added the abandoned - stale PRs automatically closed due to stale status label Mar 28, 2023
@github-actions github-actions bot closed this Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - Waiting on author Waiting for author to respond to review abandoned - stale PRs automatically closed due to stale status stale Marker label for stale issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support nogil=True for ufuncs and gufuncs
5 participants