-
Notifications
You must be signed in to change notification settings - Fork 30
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
Update empty vals check in dpt.place #1106
Update empty vals check in dpt.place #1106
Conversation
dpctl/tensor/_indexing_functions.py
Outdated
@@ -293,7 +293,7 @@ def place(arr, mask, vals): | |||
raise dpctl.utils.ExecutionPlacementError | |||
if arr.shape != mask.shape or vals.ndim != 1: | |||
raise ValueError("Array sizes are not as required") | |||
if vals.size == 0: | |||
if vals.size == 0 and dpt.nonzero(mask)[0].size: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The number of set elements in the mask is computed on line 299 and assigned to variable nz_count
.
Perhaps it is cleaner to move the check for vals.size == 0
till after nz_count
value is known?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, It looks cleaner.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you for catching the issue and providing the fix @vlad-perevezentsev
Array API standard conformance tests for dpctl=0.14.2=py310h76be34b_5 ran successfully. |
This PR complements #1105 .
The original behavior of
numpy.place
throws an exception when it gets an empty array ofvals
argumentBUT if the
mask
argument contains allFalse
,numpy.place
works without exceptions (example).