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

[Torch] Fix advanced indexing with NoneType index arguments #13826

Merged
merged 1 commit into from
Feb 2, 2023

Conversation

padreofthegame
Copy link
Contributor

@padreofthegame padreofthegame commented Jan 23, 2023

Suppose to solve issue #12922.

@tvm-bot
Copy link
Collaborator

tvm-bot commented Jan 23, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@shingjan
Copy link

Thanks for taking a look at this @padreofthegame! Will circle back once this PR is ready for review

@padreofthegame
Copy link
Contributor Author

This solution is very similar to #13306, and represents workaround for the problem mentioned in #12922 for PyTorch models (since advanced indexing does not work the same for example for numpy and for pytorch, I put solution in torch frontend, because issue was reported for torch models).
Also, I put WIP label only because I am not sure if I managed to cover all possible test cases, so @shingjan you are welcome to try some additional scenarios, and call me out if detect something strange.

@shingjan
Copy link

@padreofthegame i think this fix is fine in terms of the difference between adv_index of numpy and it of pytorch. There is no numpy frontend for TVM yet. I think overall this PR LGTM. Will be great if @masahi can take a look and approve. Thanks!

@padreofthegame padreofthegame changed the title WIP: [Torch] Fix advanced indexing with NoneType index arguments [Torch] Fix advanced indexing with NoneType index arguments Jan 27, 2023
Copy link

@shingjan shingjan left a comment

Choose a reason for hiding this comment

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

LGTM

@masahi masahi merged commit 7aecc1a into apache:main Feb 2, 2023
@padreofthegame padreofthegame deleted the pytorch/adv_index branch February 2, 2023 13:17
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.

4 participants