fix functionalize() input metadata mutations, migrate core test to use functionalize()#82601
Closed
bdhirsh wants to merge 12 commits intogh/bdhirsh/288/basefrom
Closed
fix functionalize() input metadata mutations, migrate core test to use functionalize()#82601bdhirsh wants to merge 12 commits intogh/bdhirsh/288/basefrom
bdhirsh wants to merge 12 commits intogh/bdhirsh/288/basefrom
Conversation
…e functionalize() [ghstack-poisoned]
Contributor
🔗 Helpful links
❌ 14 New Failures, 1 Flaky FailuresAs of commit 6830320 (more details on the Dr. CI page): Expand to see more
🕵️ 14 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages
|
… test to use functionalize()" [ghstack-poisoned]
ezyang
reviewed
Aug 2, 2022
| from torch.utils._pytree import tree_map | ||
| from torch.fx.experimental.proxy_tensor import make_fx | ||
| from torch.fx.passes.reinplace import reinplace | ||
| from functorch.experimental import functionalize |
Contributor
There was a problem hiding this comment.
since the build systems are not unified yet, ideally you should test if functorch is actually importable
Collaborator
Author
There was a problem hiding this comment.
oh, looks like CI is actually failing for that reason....
Any idea on the timeline for unifying build systems? Otherwise I guess I could try to import functionalize(), and fall back to the roll-your-own version that was in this file.
Contributor
There was a problem hiding this comment.
to test in CI, just update the CI to start building functorch after regular pytorch
@zou3519 knows unified build timeline
Contributor
There was a problem hiding this comment.
+1, A short fix is to install functorch in all CI configurations so that it's available for testing.
Unified build timeline: by 1.13, unless people have reasons for it earlier
ezyang
approved these changes
Aug 2, 2022
Contributor
ezyang
left a comment
There was a problem hiding this comment.
short and sweet (well maybe not the expect test updates lol)
… test to use functionalize()" Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use `as_strided_()` to mutate input metadata when necessary, similar to how it uses `copy_()` to handle input data mutations. I also switched the functionalization tests over to using the proper `functionalize()` from functorch, which resulted in a few expect test changes. [ghstack-poisoned]
… test to use functionalize()" Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use `as_strided_()` to mutate input metadata when necessary, similar to how it uses `copy_()` to handle input data mutations. I also switched the functionalization tests over to using the proper `functionalize()` from functorch, which resulted in a few expect test changes. [ghstack-poisoned]
This was referenced Aug 3, 2022
… test to use functionalize()" Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use `as_strided_()` to mutate input metadata when necessary, similar to how it uses `copy_()` to handle input data mutations. I also switched the functionalization tests over to using the proper `functionalize()` from functorch, which resulted in a few expect test changes. [ghstack-poisoned]
… test to use functionalize()" Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use `as_strided_()` to mutate input metadata when necessary, similar to how it uses `copy_()` to handle input data mutations. I also switched the functionalization tests over to using the proper `functionalize()` from functorch, which resulted in a few expect test changes. [ghstack-poisoned]
This was referenced Aug 24, 2022
Closed
bdhirsh
added a commit
that referenced
this pull request
Aug 24, 2022
…n inputs" A version of this PR was sitting at #82601 but that PR some other cleanup that relies on being able to use functorch in pytorch/pytorch CI tests, which isn't ready yet. I pulled the change out here to unblock functionalization for some models run with inductor (see pytorch/torchdynamo#964 (comment)). [ghstack-poisoned]
bdhirsh
added a commit
that referenced
this pull request
Aug 24, 2022
A version of this PR was sitting at #82601 but that PR some other cleanup that relies on being able to use functorch in pytorch/pytorch CI tests, which isn't ready yet. I pulled the change out here to unblock functionalization for some models run with inductor (see pytorch/torchdynamo#964 (comment)). [ghstack-poisoned]
bdhirsh
added a commit
that referenced
this pull request
Aug 25, 2022
…n inputs" A version of this PR was sitting at #82601 but that PR some other cleanup that relies on being able to use functorch in pytorch/pytorch CI tests, which isn't ready yet. I pulled the change out here to unblock functionalization for some models run with inductor (see pytorch/torchdynamo#964 (comment)). [ghstack-poisoned]
bdhirsh
added a commit
that referenced
this pull request
Aug 25, 2022
A version of this PR was sitting at #82601 but that PR some other cleanup that relies on being able to use functorch in pytorch/pytorch CI tests, which isn't ready yet. I pulled the change out here to unblock functionalization for some models run with inductor (see pytorch/torchdynamo#964 (comment)). [ghstack-poisoned]
pytorchmergebot
pushed a commit
that referenced
this pull request
Aug 25, 2022
A version of this PR was sitting at #82601 but that PR some other cleanup that relies on being able to use functorch in pytorch/pytorch CI tests, which isn't ready yet. I pulled the change out here to unblock functionalization for some models run with inductor (see pytorch/torchdynamo#964 (comment)). Pull Request resolved: #83993 Approved by: https://github.com/ezyang
facebook-github-bot
pushed a commit
to pytorch/functorch
that referenced
this pull request
Aug 26, 2022
Summary: A version of this PR was sitting at pytorch/pytorch#82601 but that PR some other cleanup that relies on being able to use functorch in pytorch/pytorch CI tests, which isn't ready yet. I pulled the change out here to unblock functionalization for some models run with inductor (see pytorch/torchdynamo#964 (comment)). X-link: pytorch/pytorch#83993 Approved by: https://github.com/ezyang Reviewed By: malfet Differential Revision: D39034436 Pulled By: bdhirsh fbshipit-source-id: a419018f4306e8dc2e7542d793c0520bf821fb1d
facebook-github-bot
pushed a commit
that referenced
this pull request
Aug 26, 2022
Summary: A version of this PR was sitting at #82601 but that PR some other cleanup that relies on being able to use functorch in pytorch/pytorch CI tests, which isn't ready yet. I pulled the change out here to unblock functionalization for some models run with inductor (see pytorch/torchdynamo#964 (comment)). Pull Request resolved: #83993 Approved by: https://github.com/ezyang Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/b82c74da07430ba4a221403d383eeb27de04f7f7 Reviewed By: malfet Differential Revision: D39034436 Pulled By: bdhirsh fbshipit-source-id: a419018f4306e8dc2e7542d793c0520bf821fb1d
… test to use functionalize()" Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use `as_strided_()` to mutate input metadata when necessary, similar to how it uses `copy_()` to handle input data mutations. I also switched the functionalization tests over to using the proper `functionalize()` from functorch, which resulted in a few expect test changes. [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/82601
Note: Links to docs will display an error until the docs builds have been completed. ❌ 7 Failures, 1 PendingAs of commit a75a5f2: The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
… test to use functionalize()" Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use `as_strided_()` to mutate input metadata when necessary, similar to how it uses `copy_()` to handle input data mutations. I also switched the functionalization tests over to using the proper `functionalize()` from functorch, which resulted in a few expect test changes. [ghstack-poisoned]
… test to use functionalize()" Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use `as_strided_()` to mutate input metadata when necessary, similar to how it uses `copy_()` to handle input data mutations. I also switched the functionalization tests over to using the proper `functionalize()` from functorch, which resulted in a few expect test changes. [ghstack-poisoned]
… test to use functionalize()" Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use `as_strided_()` to mutate input metadata when necessary, similar to how it uses `copy_()` to handle input data mutations. I also switched the functionalization tests over to using the proper `functionalize()` from functorch, which resulted in a few expect test changes. [ghstack-poisoned]
… test to use functionalize()" Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use `as_strided_()` to mutate input metadata when necessary, similar to how it uses `copy_()` to handle input data mutations. I also switched the functionalization tests over to using the proper `functionalize()` from functorch, which resulted in a few expect test changes. [ghstack-poisoned]
Closed
… test to use functionalize()" Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use `as_strided_()` to mutate input metadata when necessary, similar to how it uses `copy_()` to handle input data mutations. I also switched the functionalization tests over to using the proper `functionalize()` from functorch, which resulted in a few expect test changes. [ghstack-poisoned]
Contributor
|
/easycla As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details. This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign. |
Contributor
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Although this won't fully solve the "input metadata mutations" issue for AOTAutograd, there's no reason functionalization should break on input metadata mutations - this PR updates functionalization to use
as_strided_()to mutate input metadata when necessary, similar to how it usescopy_()to handle input data mutations.I also switched the functionalization tests over to using the proper
functionalize()from functorch, which resulted in a few expect test changes.Stack from ghstack: