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

Type promotion for indices arrays and casting vals in integer indexing #1647

Merged
merged 6 commits into from
Apr 21, 2024

Conversation

ndgrigorian
Copy link
Collaborator

This pull request proposes resolutions to #1360, #1382, and #1482 by changing the behavior of advanced indexing and indexing functions.

  • x[i_0, i_1, ...] now promotes arrays i_0 ... i_N to an appropriate integer data type, and only raises where such a data type cannot be found
  • x[indices] = vals now casts vals to the data type of x regardless of type. This aligns with place/boolean indexing.
  • dpt.put now warns the user about race conditions when indices are not unique

Additionally, this PR implements changes to take, put, and generalized advanced integer indexing which handle cases where empty axes are being indexed.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • If this PR is a work in progress, are you opening the PR as a draft?

Setting items in an array now casts the right-hand side to the array data type when the data types differ

Setting and getting from an empty axis with non-empty indices now throws `IndexError`
Fixes `take` and `put` being used on non-empty axes with non-empty indices

Also adds a note to `put` about race conditions for non-unique indices
Also corrects error raised in _put_multi_index when attempting to put into indices along an empty axis
Copy link

github-actions bot commented Apr 19, 2024

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

Copy link

Array API standard conformance tests for dpctl=0.17.0dev0=py310h15de555_305 ran successfully.
Passed: 870
Failed: 8
Skipped: 92

@coveralls
Copy link
Collaborator

coveralls commented Apr 19, 2024

Coverage Status

coverage: 88.21% (+0.03%) from 88.177%
when pulling 6b3b099 on integer-indexing-casting
into f5c6610 on master.

Copy link
Collaborator

@oleksandr-pavlyk oleksandr-pavlyk left a comment

Choose a reason for hiding this comment

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

Needed change overall! Two small nits :)

Thanks for working on this @ndgrigorian !

Copy link

Array API standard conformance tests for dpctl=0.17.0dev0=py310h15de555_306 ran successfully.
Passed: 870
Failed: 8
Skipped: 92

Copy link
Collaborator

@oleksandr-pavlyk oleksandr-pavlyk left a comment

Choose a reason for hiding this comment

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

LGTM!

@ndgrigorian ndgrigorian merged commit 7757857 into master Apr 21, 2024
54 of 60 checks passed
@ndgrigorian ndgrigorian deleted the integer-indexing-casting branch May 7, 2024 08:34
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