Skip to content

towards addressing issue 472#477

Closed
ipdemes wants to merge 2 commits intonv-legate:branch-22.07from
ipdemes:issue_472
Closed

towards addressing issue 472#477
ipdemes wants to merge 2 commits intonv-legate:branch-22.07from
ipdemes:issue_472

Conversation

@ipdemes
Copy link
Contributor

@ipdemes ipdemes commented Jul 28, 2022

Addressing #472
I am not sure if this is the best solution because it will inline_map bounds of the slice. @wonchan, @manopapad : do you suggest any better idea?

@ipdemes ipdemes requested review from magnatelee and manopapad July 28, 2022 04:04
def _convert_key(self, key, first=True):
# Convert any arrays stored in a key to a cuNumeric array
if isinstance(key, slice):
if isinstance(key.start, ndarray):
Copy link
Contributor

@magnatelee magnatelee Jul 28, 2022

Choose a reason for hiding this comment

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

I'd suggest you do the integer conversion unconditionally. An isinstance call isn't particularly cheaper. Plus the code would look cleaner in that way.

@manopapad
Copy link
Contributor

I have another attempt at this, using the operator.index(.) method instead of int(), as the python documentation suggests, and handling some additional cases (also support indexing numpy ndarrays and python lists): #479

@ipdemes
Copy link
Contributor Author

ipdemes commented Jul 29, 2022

closing this PR as #479 addresses the issue

@ipdemes ipdemes closed this Jul 29, 2022
@ipdemes ipdemes deleted the issue_472 branch August 2, 2022 17:33
manopapad pushed a commit that referenced this pull request Nov 17, 2024
* cuNumeric -> cuPyNumeric

* restore .dockerigore link

* update legate commit hash
XiaLuNV pushed a commit to XiaLuNV/cunumeric that referenced this pull request Dec 13, 2024
* cuNumeric -> cuPyNumeric

* restore .dockerigore link

* update legate commit hash
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