KEMBAR78

[core IR] Add decomps for `aten.sum` and `aten.squeeze` variants by SS-JIA · Pull Request #110645 · pytorch/pytorch · GitHub

[core IR] Add decomps for `aten.sum` and `aten.squeeze` variants #110645

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.

Sign up for GitHub

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

Jump to bottom

Closed

SS-JIA wants to merge 1 commit into pytorch:main from SS-JIA:export-D49965952

Contributor

SS-JIA commented Oct 5, 2023 •

edited

Loading

Summary:

Context

Both aten.sum and aten.squeeze have a "most generic" variant in the form of aten.sum.dim_IntList and aten.squeeze.dims respectively. Add decompositions for other non generic variants of these operators to express them using the most generic variant.

Note that to register these decomps, the reference implementation under _refs had to be removed as registered decompositions. cc: @lezcano @peterbell10

Test Plan: Github CI + Meta Internal CI

Differential Revision: D49965952

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

pytorch-bot bot commented Oct 5, 2023 •

edited

Loading

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 382eeca with merge base 17348b0 ():

UNSTABLE - The following job failed but was likely due to flakiness present on trunk and has been marked as unstable:

trunk / linux-focal-rocm5.6-py3.8 / test (default, 1, 3, linux.rocm.gpu, unstable) (gh)

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

github-actions bot added module: inductor ciflow/inductor labels

Contributor

facebook-github-bot commented Oct 5, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

facebook-github-bot added the fb-exported label

SS-JIA requested review from SherlockNoMad, bdhirsh, kimishpatel, lezcano and peterbell10

October 5, 2023 20:56

peterbell10 reviewed

View reviewed changes

torch/_refs/__init__.py Outdated Show resolved Hide resolved

SS-JIA force-pushed the export-D49965952 branch from b9e8e2a to 5ee7630 Compare

October 5, 2023 21:18

Contributor

facebook-github-bot commented Oct 5, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

SS-JIA force-pushed the export-D49965952 branch from 5ee7630 to 3db9dcd Compare

October 5, 2023 21:19

Contributor

facebook-github-bot commented Oct 5, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

peterbell10 reviewed

View reviewed changes

test/test_decomp.py Outdated Show resolved Hide resolved

SS-JIA force-pushed the export-D49965952 branch from 3db9dcd to 526c734 Compare

October 5, 2023 21:28

Contributor

facebook-github-bot commented Oct 5, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

SS-JIA pushed a commit to SS-JIA/pytorch that referenced this pull request


          [core IR] Add decomps for aten.sum and aten.squeeze variants (pyt…

6b62792

…orch#110645)

Summary:

## Context

Both `aten.sum` and `aten.squeeze` have a "most generic" variant in the form of `aten.sum.dim_IntList` and `aten.squeeze.dims` respectively. Add decompositions for other non generic variants of these operators to express them using the most generic variant.

Note that to register these decomps, the reference implementation under `_refs` had to be removed as registered decompositions.

Test Plan: Github CI + Meta Internal CI

Differential Revision: D49965952

SS-JIA force-pushed the export-D49965952 branch from 526c734 to 6b62792 Compare

October 5, 2023 21:28

Contributor

facebook-github-bot commented Oct 5, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

SS-JIA force-pushed the export-D49965952 branch from 6b62792 to 4cb1610 Compare

October 6, 2023 00:48

SS-JIA force-pushed the export-D49965952 branch from 4cb1610 to 8dcc266 Compare

October 6, 2023 00:49

Contributor

facebook-github-bot commented Oct 6, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

1 similar comment

Contributor

facebook-github-bot commented Oct 6, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

SS-JIA force-pushed the export-D49965952 branch from 9bde7e1 to d02c8b3 Compare

October 6, 2023 12:49

Contributor

facebook-github-bot commented Oct 6, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

1 similar comment

Contributor

facebook-github-bot commented Oct 6, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

Contributor Author

SS-JIA commented Oct 6, 2023

Updated to address comments.

Contributor

facebook-github-bot commented Oct 6, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

SS-JIA force-pushed the export-D49965952 branch from d02c8b3 to 23779e8 Compare

October 6, 2023 12:54

SS-JIA pushed a commit to SS-JIA/pytorch that referenced this pull request


          [core IR] Add decomps for aten.sum and aten.squeeze variants (pyt…

39b0345

…orch#110645)

Summary:

## Context

Both `aten.sum` and `aten.squeeze` have a "most generic" variant in the form of `aten.sum.dim_IntList` and `aten.squeeze.dims` respectively. Add decompositions for other non generic variants of these operators to express them using the most generic variant.

Note that to register these decomps, the reference implementation under `_refs` had to be removed as registered decompositions.

Test Plan: Github CI + Meta Internal CI

Differential Revision: D49965952

SS-JIA force-pushed the export-D49965952 branch from 23779e8 to 39b0345 Compare

October 6, 2023 12:56

Contributor

facebook-github-bot commented Oct 6, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

SS-JIA added the release notes: export label

peterbell10 approved these changes

View reviewed changes

lezcano reviewed

View reviewed changes

Collaborator

lezcano left a comment

These two were already being decomposed by the ref. It's a bit annoying to have to factor them out into their own decompositions. I think a proper fix going forward would be to fix the issues with prims: Either make the usable (removing the broadcasting behaviour and dispatching them to the core ATen ops) or remove them completely. Otherwise, we are going to end up with plenty of these tiny decompositions, rather than the tighter all-in-one that we have implemented in refs.

digantdesai approved these changes

View reviewed changes

manuelcandales approved these changes

View reviewed changes


          [core IR] Add decomps for aten.sum and aten.squeeze variants (pyt…

382eeca

…orch#110645)

Summary:

## Context

Both `aten.sum` and `aten.squeeze` have a "most generic" variant in the form of `aten.sum.dim_IntList` and `aten.squeeze.dims` respectively. Add decompositions for other non generic variants of these operators to express them using the most generic variant.

Note that to register these decomps, the reference implementation under `_refs` had to be removed as registered decompositions.

Test Plan: Github CI + Meta Internal CI

Reviewed By: manuelcandales

Differential Revision: D49965952

SS-JIA force-pushed the export-D49965952 branch from 39b0345 to 7583f15 Compare

October 6, 2023 18:45

Contributor

facebook-github-bot commented Oct 6, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

SS-JIA force-pushed the export-D49965952 branch from 7583f15 to 382eeca Compare

October 6, 2023 18:45

Contributor

facebook-github-bot commented Oct 6, 2023

This pull request was exported from Phabricator. Differential Revision: D49965952

Contributor

facebook-github-bot commented Oct 7, 2023

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

pytorch-bot bot added the ciflow/trunk label

pytorchmergebot added the merging label

Collaborator

pytorchmergebot commented Oct 7, 2023

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

pytorchmergebot added Merged and removed merging labels

pytorchmergebot closed this in

c2e7a0d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lezcano lezcano left review comments

digantdesai digantdesai approved these changes

peterbell10 peterbell10 approved these changes

manuelcandales manuelcandales approved these changes

SherlockNoMad Awaiting requested review from SherlockNoMad

bdhirsh Awaiting requested review from bdhirsh

kimishpatel Awaiting requested review from kimishpatel

Labels

ciflow/inductor ciflow/trunk fb-exported Merged module: inductor release notes: export