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

Implement tensor slice in inductor to stop falling back for aten.index #111015

Closed
wants to merge 5 commits into from

Conversation

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 11, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/111015

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (3 Unrelated Failures)

As of commit d8b9539 with merge base 144cda7 (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

oulgen added a commit that referenced this pull request Oct 11, 2023
Fixes #110711

ghstack-source-id: 29647203b007c5985741bd2bb8dcaca990caa490
Pull Request resolved: #111015
@oulgen oulgen requested a review from jansel October 11, 2023 02:03
@oulgen oulgen requested review from Chillee and ezyang October 11, 2023 02:03
@oulgen oulgen added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 11, 2023
@oulgen oulgen changed the title Implement tensor slice in to stop falling back for aten.index Implement tensor slice in inductor to stop falling back for aten.index Oct 11, 2023
…r aten.index"

Fixes #110711

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler

[ghstack-poisoned]
oulgen added a commit that referenced this pull request Oct 11, 2023
Fixes #110711

ghstack-source-id: c88c75628750057420db135bb624cb6dc8b02337
Pull Request resolved: #111015
…r aten.index"

Fixes #110711

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler

[ghstack-poisoned]
oulgen added a commit that referenced this pull request Oct 11, 2023
Fixes #110711

ghstack-source-id: 68f9c591836ea6b71a661e75031d027ab2ab45da
Pull Request resolved: #111015
@ezyang ezyang removed their request for review October 11, 2023 02:51
@ezyang
Copy link
Contributor

ezyang commented Oct 11, 2023

I'm not terribly familiar with this code but I can page it in if you really need me to.

@oulgen
Copy link
Contributor Author

oulgen commented Oct 11, 2023

@ezyang No worries, @Chillee and I were trying to figure out the right semantics for tensor slicing, wanted some extra eyes to make sure we did not miss an edge case.

…r aten.index"

Fixes #110711

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler

[ghstack-poisoned]
oulgen added a commit that referenced this pull request Oct 11, 2023
Fixes #110711

ghstack-source-id: 271a33fc13a1a302ed614b79206a7dfac7714b4a
Pull Request resolved: #111015
@oulgen
Copy link
Contributor Author

oulgen commented Oct 11, 2023

test/inductor/test_torchinductor.py Show resolved Hide resolved


def index_output_size_and_inner_fn(
x_size, indices, valid_idxs, tensor_size, indices_loaders, indexed_size, x_loader
Copy link
Contributor

Choose a reason for hiding this comment

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

I think valid_idxs is a bit of a bad name. Perhaps "tensor_indices"?

Copy link
Contributor

Choose a reason for hiding this comment

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

In general I think the mixup of idxs and indices is a bit confusing.

torch/_inductor/lowering.py Show resolved Hide resolved
…r aten.index"

Fixes #110711

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler

[ghstack-poisoned]
oulgen added a commit that referenced this pull request Oct 11, 2023
Fixes #110711

ghstack-source-id: fa80c24abde1cdec4c126de71f8885120fb81fcf
Pull Request resolved: #111015
@oulgen
Copy link
Contributor Author

oulgen commented Oct 11, 2023

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@facebook-github-bot facebook-github-bot deleted the gh/oulgen/6/head branch November 11, 2023 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants