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

Reland "Update AOTAutograd to use FunctionalTensorMode instead of C++ functionalization (#106406)" #109906

Closed
wants to merge 1 commit into from

Conversation

bdhirsh
Copy link
Contributor

@bdhirsh bdhirsh commented Sep 22, 2023

I'm pretty sure this is fixed but I'll run inductor and trunk CI. The failing test in trunk previously was that the selective activation checkpointing code that landed recently assumes that it can detect whether or not AOTAutograd is running by seeing if the inputs to SAC are C++ FunctionalTensorWrappers

Stack from ghstack (oldest at bottom):

previous land broke some inductor trunk tests

This reverts commit 629a628.

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

… functionalization (#106406)"

previous land broke some inductor trunk tests

This reverts commit 629a628.

[ghstack-poisoned]
@bdhirsh bdhirsh requested review from ezyang, Chillee and a team as code owners September 22, 2023 19:33
@pytorch-bot pytorch-bot bot added the release notes: releng release notes category label Sep 22, 2023
@pytorch-bot
Copy link

pytorch-bot bot commented Sep 22, 2023

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit e349469 with merge base d0c8e82 (image):

NEW FAILURE - The following job has failed:

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

bdhirsh added a commit that referenced this pull request Sep 22, 2023
… functionalization (#106406)"

previous land broke some inductor trunk tests

This reverts commit 629a628.

ghstack-source-id: 61eebc1d4b2a1d915a3f26b153ff9b73ce7f1dd8
Pull Request resolved: #109906
@bdhirsh bdhirsh added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 22, 2023
@bdhirsh
Copy link
Contributor Author

bdhirsh commented Sep 25, 2023

@pytorchbot merge -f "flaky rocm test: https://github.com/pytorch/pytorch/actions/runs/6278467379/job/17052810190"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

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

@atalman
Copy link
Contributor

atalman commented Sep 26, 2023

@pytorchbot revert -m "Breaks internal tests" -c ghfirst

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Collaborator

@bdhirsh your PR has been successfully reverted.

pytorchmergebot added a commit that referenced this pull request Sep 26, 2023
bdhirsh added a commit that referenced this pull request Sep 26, 2023
…nstead of C++ functionalization (#106406)" (#109906)"

This reverts commit 1b90f07.

ghstack-source-id: 934adbbafd464a5b51217d4bfc1ea9077b6e3e04
Pull Request resolved: #110079
bdhirsh added a commit that referenced this pull request Sep 26, 2023
…nstead of C++ functionalization (#106406)" (#109906)"

This reverts commit 1b90f07.

[ghstack-poisoned]
@facebook-github-bot facebook-github-bot deleted the gh/bdhirsh/466/head branch September 29, 2023 14:23
bdhirsh added a commit that referenced this pull request Sep 29, 2023
…nstead of C++ functionalization (#106406)" (#109906)"

This reverts commit 1b90f07.

ghstack-source-id: 58d5745aa2ad70ef308e35d33736c572446368c0
Pull Request resolved: #110079
bdhirsh added a commit that referenced this pull request Sep 29, 2023
… use FunctionalTensorMode instead of C++ functionalization (#106406)" (#109906)""

The first reland broke internal (failing diff: D49617462).

The major error looks like it's because there's an internal-only higher order op that needs a new functionalization rule. I'm going to land an internal diff for that and confirm tests pass before relanding this PR.


This reverts commit 1b90f07.

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

[ghstack-poisoned]
bdhirsh added a commit that referenced this pull request Sep 29, 2023
…ensorMode instead of C++ functionalization (#106406)" (#109906)""

The first reland broke internal (failing diff: D49617462).

The major error looks like it's because there's an internal-only higher order op that needs a new functionalization rule. I'm going to land an internal diff for that and confirm tests pass before relanding this PR.


This reverts commit 1b90f07.

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

[ghstack-poisoned]
bdhirsh added a commit that referenced this pull request Oct 3, 2023
… use FunctionalTensorMode instead of C++ functionalization (#106406)" (#109906)""

The first reland broke internal (failing diff: D49617462).

The major error looks like it's because there's an internal-only higher order op that needs a new functionalization rule. I'm going to land an internal diff for that and confirm tests pass before relanding this PR.

Also confirmed that the issue from #110121 is fixed, and added a test.


This reverts commit 1b90f07.

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

[ghstack-poisoned]
bdhirsh added a commit that referenced this pull request Oct 3, 2023
…nstead of C++ functionalization (#106406)" (#109906)"

This reverts commit 1b90f07.

ghstack-source-id: 6fed21fd7591db317f6de0efad131ae84b274e83
Pull Request resolved: #110079
bdhirsh added a commit that referenced this pull request Oct 3, 2023
…ensorMode instead of C++ functionalization (#106406)" (#109906)""

The first reland broke internal (failing diff: D49617462).

The major error looks like it's because there's an internal-only higher order op that needs a new functionalization rule. I'm going to land an internal diff for that and confirm tests pass before relanding this PR.

Also confirmed that the issue from #110121 is fixed, and added a test.


This reverts commit 1b90f07.

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

[ghstack-poisoned]
pytorchmergebot pushed a commit that referenced this pull request Oct 3, 2023
…nstead of C++ functionalization (#106406)" (#109906)" (#110079)

The first reland broke internal (failing diff: D49617462).

The major error looks like it's because there's an internal-only higher order op that needs a new functionalization rule. I'm going to land an internal diff for that and confirm tests pass before relanding this PR.

Also confirmed that the issue from #110121 is fixed, and added a test.

This reverts commit 1b90f07.

Pull Request resolved: #110079
Approved by: https://github.com/ezyang
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.

4 participants