Skip to content

Advanced indexing - #235

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

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

Conversation

@ipdemes

@ipdemes ipdemes commented Mar 25, 2022

Copy link
Copy Markdown
Contributor

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
Comment thread cunumeric/runtime.py Outdated
Comment thread test.py
Comment thread cunumeric/config.py Outdated
@manopapad

Copy link
Copy Markdown
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

ipdemes commented Apr 5, 2022

Copy link
Copy Markdown
Contributor Author

@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
Comment thread cunumeric/config.py Outdated
Comment thread src/cunumeric/index/zip_template.inl Outdated
Comment thread src/cunumeric/index/zip_template.inl Outdated
Comment thread src/cunumeric/index/zip.cc Outdated
Comment thread src/cunumeric/index/zip.cc Outdated
Comment thread src/cunumeric/index/zip.cc
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.
Comment thread src/cunumeric/cunumeric_c.h Outdated
@ipdemes
ipdemes force-pushed the advanced_indexing branch from 8e6b2bb to 95ae53d Compare April 20, 2022 00:06
@ipdemes

ipdemes commented Apr 20, 2022

Copy link
Copy Markdown
Contributor Author

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

Comment thread cunumeric/deferred.py Outdated
Comment thread cunumeric/deferred.py
Comment thread cunumeric/deferred.py Outdated
Comment thread src/cunumeric/index/zip.cu Outdated
Comment thread cunumeric/deferred.py Outdated
Comment thread cunumeric/deferred.py Outdated
Comment thread src/cunumeric/index/advanced_indexing.cu
Comment thread src/cunumeric/index/zip.cu
@manopapad

Copy link
Copy Markdown
Contributor

This is ready to merge. Great work!

@ipdemes

ipdemes commented Apr 21, 2022

Copy link
Copy Markdown
Contributor Author

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