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.

Sign up for GitHub

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

Jump to bottom

Update integer advanced indexing for array API 2024.12 spec #2032

Merged
merged 13 commits into from
Apr 9, 2025
Merged

Update integer advanced indexing for array API 2024.12 spec #2032

merged 13 commits into from
Apr 9, 2025

Conversation

ndgrigorian
Copy link
Collaborator

@ndgrigorian ndgrigorian commented Mar 26, 2025 •
edited
Loading

The array API specification now requires advanced integer indexing (i.e., with arrays) to promote Python integer scalars to arrays when mixed with integer arrays.

This PR implements this behavior, promoting any integer indices that occur in a sequence with array indices of dimension 1 or greater.

  • 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?
  • Have you added documentation for your changes, if necessary?
  • Have you added your changes to the changelog?
  • If this PR is a work in progress, are you opening the PR as a draft?

Sorry, something went wrong.

@ndgrigorian ndgrigorian marked this pull request as draft March 26, 2025 23:15
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 26, 2025 •
edited
Loading

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

@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_75 ran successfully.
Passed: 894
Failed: 2
Skipped: 154

@coveralls
Copy link
Collaborator

coveralls commented Mar 26, 2025 •
edited
Loading

Coverage Status

coverage: 86.392% (+0.01%) from 86.381%
when pulling 2fdac4c on update-indexing-for-array-api-2024
into f8c751b on master.

@ndgrigorian ndgrigorian force-pushed the update-indexing-for-array-api-2024 branch from 619dbfd to a02b5e5 Compare March 31, 2025 16:36
@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_78 ran successfully.
Passed: 894
Failed: 2
Skipped: 154

@ndgrigorian ndgrigorian marked this pull request as ready for review March 31, 2025 23:23
antonwolfy
antonwolfy reviewed Apr 1, 2025
View reviewed changes
@ndgrigorian ndgrigorian force-pushed the update-indexing-for-array-api-2024 branch from a02b5e5 to 71ca729 Compare April 2, 2025 06:49
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 2, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_104 ran successfully.
Passed: 894
Failed: 2
Skipped: 154

@ndgrigorian ndgrigorian force-pushed the update-indexing-for-array-api-2024 branch from 71ca729 to d426408 Compare April 3, 2025 01:57
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 3, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_109 ran successfully.
Passed: 894
Failed: 2
Skipped: 154

@ndgrigorian ndgrigorian requested a review from antonwolfy April 3, 2025 05:53
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 3, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_110 ran successfully.
Passed: 894
Failed: 2
Skipped: 154

@ndgrigorian
Update integer advanced indexing for array API 2024
87fa69a 
array API 2024 spec requires advanced indexing with integer arrays, and when advanced indexing behavior is triggered, integral scalars are to be converted to arrays and broadcast with the other indices
@ndgrigorian
only treat integers as arrays when arrays have been consecutive
47639da 
this fixes cases like `x[..., arr1, int1, arr2, int2, :, int3]`, the final int will be treated as an integer instead of an array, and `arr1` through `int2` will be treated as arrays
@ndgrigorian
raise for OOB indices even if they will be converted to arrays
b18fe9d 
since the index is on the host, we can raise like we normally would, which follows NumPy behavior
@ndgrigorian
Add common utilities for _put_multi_index and _take_multi_index
9a7705e 
Reduces code duplication
@ndgrigorian
Remove redundant IndexError in _slicing.pxi
25fb1a2
@ndgrigorian
Fix over-indentation
14a55d0 
Typos from advanced indexing updates
@ndgrigorian
Add tests for uncovered branches in advanced indexing
72f8938
@ndgrigorian
Add another test for advanced indexing
b38478b 
Test for the case where an basic integral index appears between two integral arrays, followed by a basic index, and then followed by `:`
@ndgrigorian
Add another test for advanced indexing errors
d873f35
@ndgrigorian ndgrigorian force-pushed the update-indexing-for-array-api-2024 branch from 12e2121 to d873f35 Compare April 8, 2025 01:34
@ndgrigorian
Copy link
Collaborator Author

@antonwolfy
a couple of new tests added, I think PR is ready for another review pass

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 8, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_116 ran successfully.
Passed: 894
Failed: 2
Skipped: 154

antonwolfy
antonwolfy reviewed Apr 8, 2025
View reviewed changes
@ndgrigorian
Partial reversion of advanced indexing changes
0784d70 
0D array indices are treated as Python scalars and moved to the host when not part of other, consecutive array indices

This means that 0D arrays also can't start a series of consecutive advanced indices
@ndgrigorian ndgrigorian requested a review from antonwolfy April 9, 2025 03:27
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 9, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_123 ran successfully.
Passed: 912
Failed: 2
Skipped: 154

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 9, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_126 ran successfully.
Passed: 913
Failed: 1
Skipped: 154

@ndgrigorian ndgrigorian requested a review from antonwolfy April 9, 2025 16:56
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 9, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_128 ran successfully.
Passed: 912
Failed: 2
Skipped: 154

antonwolfy
antonwolfy approved these changes Apr 9, 2025
View reviewed changes
Copy link
Collaborator

@antonwolfy antonwolfy left a comment

Choose a reason for hiding this comment

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

Thank you @ndgrigorian, LGTM

@ndgrigorian ndgrigorian merged commit 4f63b18 into master Apr 9, 2025
72 of 73 checks passed
@ndgrigorian ndgrigorian deleted the update-indexing-for-array-api-2024 branch April 9, 2025 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antonwolfy antonwolfy

@vlad-perevezentsev vlad-perevezentsev

@vtavana vtavana

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ndgrigorian @coveralls @antonwolfy