Skip to content

Advanced indexing#235

Merged
ipdemes merged 36 commits intonv-legate:branch-22.05from
ipdemes:advanced_indexing
Apr 21, 2022
Merged

Advanced indexing#235
ipdemes merged 36 commits intonv-legate:branch-22.05from
ipdemes:advanced_indexing

Conversation

@ipdemes
Copy link
Contributor

@ipdemes ipdemes commented Mar 25, 2022

Adding support for advanced indexing

@ipdemes ipdemes requested a review from manopapad March 25, 2022 04:03
@ipdemes ipdemes changed the base branch from branch-22.03 to branch-22.05 March 25, 2022 15:12
@manopapad
Copy link
Contributor

Note that, when deciding where to put the extra dimensions in a partial-basic, partial-advanced expression, NumPy considers constants as advanced arrays, so if it sees something like x[:,ind,:,1] it considers it two advanced arrays separated by :, and puts the added dimensions in the beginning:

import numpy as np
import cunumeric as cn
def foo(lib):
    x = lib.ones((3,4,5,6))
    ind = lib.ones((2,2), dtype=int)
    return x[:,ind,:,1].shape
print(foo(np))
print(foo(cn))

prints:

(2, 2, 3, 5)
(3, 2, 2, 5)

@ipdemes
Copy link
Contributor Author

ipdemes commented Apr 5, 2022

@manopapad : thank you for finding this bug. I will push the fix shortly

@ipdemes ipdemes force-pushed the advanced_indexing branch from 61e6847 to acdae9d Compare April 7, 2022 16:54
In the case when input arrays (index arrays) are
broadcasted and, legate not always will partition them
(it will sometimes just broadcast them). This doesn't
effect correctness.
@ipdemes ipdemes force-pushed the advanced_indexing branch from 8e6b2bb to 95ae53d Compare April 20, 2022 00:06
@ipdemes
Copy link
Contributor Author

ipdemes commented Apr 20, 2022

On the most recent checkout the index_routines.py test is failing on my local Mac, with:

I have fixed it by basically removing this check. In some cases, when index array is promoted at the front of the array, Legate will decide not to partition it and, instead, broadcast it to all index points. This doesn't change the logic in the task and doesn't effect correctness
@manopapad

@manopapad
Copy link
Contributor

This is ready to merge. Great work!

@ipdemes
Copy link
Contributor Author

ipdemes commented Apr 21, 2022

This is ready to merge. Great work!

Thank you for your help!

@ipdemes ipdemes merged commit bdbd205 into nv-legate:branch-22.05 Apr 21, 2022
@ipdemes ipdemes deleted the advanced_indexing branch May 26, 2022 20:20
ipdemes pushed a commit to ipdemes/cunumeric that referenced this pull request Jun 7, 2022
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