Change export IR to non-functional pre-dispatch IR #139511

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

tugsbayasgalan wants to merge 32 commits into gh/tugsbayasgalan/272/base from gh/tugsbayasgalan/272/head

Contributor

tugsbayasgalan commented Nov 1, 2024 •

edited

Loading

Stack from ghstack (oldest at bottom):

-> Change export IR to non-functional pre-dispatch IR #139511

Differential Revision: D65362160

State after this IR:

For the tests that require inference IR, they are replaced with ep.run_decomp({}) so export_for_training_run_decomp is sort of redundant but i guess it is still nice that multiple round of retracing still working. In general, we need some auditing to reduce our redundant testing coverages.
After this PR landed and not get reverted for a week or so, i will replace the export_for_training calls with export as they are the same thing now.
Added more tests to also cover now "deprecated" old IR by patching export to use old export. For reviewers, please look at the internal version.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames


          TEMP enable

939f163

[ghstack-poisoned]

tugsbayasgalan requested review from angelayi, avikchaudhuri, ydwu4 and zhxchen17 as code owners

November 1, 2024 20:13

This was referenced Nov 1, 2024

Make test_torchbind.py training IR compatible #138658

Closed

Fix custom obj being input #139209

Closed

pytorch-bot bot commented Nov 1, 2024 •

edited

Loading

🔗 Helpful Links

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

📄 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.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

[DomainsOnly] Jobs fail with GLIBC version not found

❌ 1 New Failure

As of commit 84d1538 with merge base 9bc9d4c ():

NEW FAILURE - The following job has failed:

Lint / lintrunner-noclang / linux-job (gh)
>>> Lint for test/test_nestedtensor.py:

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

tugsbayasgalan mentioned this pull request

Move pippy to training IR #139233

Closed

pytorch-bot bot added the release notes: export label

tugsbayasgalan added a commit that referenced this pull request


          TEMP enable

6ac3c9d

ghstack-source-id: 35c8b6b
Pull Request resolved: #139511


          Update on "TEMP enable"

1b8515b

[ghstack-poisoned]


          Update on "TEMP enable"

7aae0e0

[ghstack-poisoned]

tugsbayasgalan added the keep-going label

Contributor Author

tugsbayasgalan commented Nov 1, 2024

@tugsbayasgalan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

pytorch-bot bot added the ciflow/trunk label

tugsbayasgalan added ciflow/trunk and removed ciflow/trunk labels


          Update on "TEMP enable"

21a797f

Differential Revision: [D65362160](https://our.internmc.facebook.com/intern/diff/D65362160)

[ghstack-poisoned]

tugsbayasgalan added a commit that referenced this pull request


          TEMP enable

da73bd5

ghstack-source-id: 25c8c46
Pull Request resolved: #139511

Contributor Author

tugsbayasgalan commented Nov 1, 2024

@tugsbayasgalan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.


          Update on "TEMP enable"

4d670fc

Differential Revision: [D65362160](https://our.internmc.facebook.com/intern/diff/D65362160)

[ghstack-poisoned]


          Update on "TEMP enable"

b13aec5

Differential Revision: [D65362160](https://our.internmc.facebook.com/intern/diff/D65362160)

[ghstack-poisoned]


          Update on "TEMP enable"

925b824

Differential Revision: [D65362160](https://our.internmc.facebook.com/intern/diff/D65362160)

[ghstack-poisoned]


          Update on "TEMP enable"

620d245

Differential Revision: [D65362160](https://our.internmc.facebook.com/intern/diff/D65362160)

[ghstack-poisoned]

Contributor

facebook-github-bot commented Nov 20, 2024

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

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

19 similar comments

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

Contributor

facebook-github-bot commented Nov 20, 2024

@pytorchbot merge -i

(Initiating merge automatically since Phabricator Diff has merged, merging with -i because oss signals were bypassed internally)

pytorchmergebot added the merging label

Collaborator

pytorchmergebot commented Nov 20, 2024

Merge started

Your change will be merged while ignoring the following 1 checks: Lint / lintrunner-noclang / linux-job

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 closed this in

87f9c1a

pytorchmergebot added Merged and removed merging labels

pobin6 pushed a commit to pobin6/pytorch that referenced this pull request


          Change export IR to non-functional pre-dispatch IR (pytorch#139511)

4ab0815

Differential Revision: [D65362160](https://our.internmc.facebook.com/intern/diff/D65362160)

State after this IR:
1. For the tests that require inference IR, they are replaced with ep.run_decomp({}) so export_for_training_run_decomp is sort of redundant but i guess it is still nice that multiple round of retracing still working. In general, we need some auditing to reduce our redundant testing coverages.
2. After this PR landed and not get reverted for a week or so, i will replace the export_for_training calls with export as they are the same thing now.
3. Added more tests to also cover now "deprecated" old IR by patching export to use old export. For reviewers, please look at the internal version.

Pull Request resolved: pytorch#139511
Approved by: https://github.com/ydwu4, https://github.com/angelayi, https://github.com/avikchaudhuri

github-actions bot deleted the gh/tugsbayasgalan/272/head branch

December 21, 2024 02:05

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

Labels

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