[cond] inlining into one of the branches when pred is a python constant #128709
+212
−119
Conversation
This file contains 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
[ghstack-poisoned]
…thon constant" When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang [ghstack-poisoned]
ydwu4
added a commit
that referenced
this pull request
Jun 14, 2024
ghstack-source-id: 6204b597c43c266c4c4bd42dfe232e25c924f8a1 Pull Request resolved: #128709
…thon constant" When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang [ghstack-poisoned]
ydwu4
added a commit
that referenced
this pull request
Jun 14, 2024
ghstack-source-id: 22661aa10d3efb61ab5b5fe840e3dac4d31fd683 Pull Request resolved: #128709
…thon constant" When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang [ghstack-poisoned]
…thon constant" When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang [ghstack-poisoned]
ydwu4
added a commit
that referenced
this pull request
Jun 17, 2024
ghstack-source-id: 289481af89844ad078f5c2540e1eac53792d1960 Pull Request resolved: #128709
angelayi
reviewed
Jun 18, 2024
torch/_higher_order_ops/cond.py
Outdated
| if isinstance(pred, (bool, int, float)): | ||
| log.warning( | ||
| "Pred is a Python constant. When used with torch.cond, it executes only one of the branches." | ||
| " If you want torch.cond to perserve two branches, please make the predicate a boolean tensor." |
There was a problem hiding this comment.
yeah, add a test_export_cond_symbool_pred to reflect this. cond won't specialize on symbool.
…thon constant" When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang [ghstack-poisoned]
ydwu4
added a commit
that referenced
this pull request
Jun 20, 2024
ghstack-source-id: ee91520c5d4cb73cdd9cb98404313d2616f59cff Pull Request resolved: #128709
…thon constant" When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang [ghstack-poisoned]
ydwu4
added a commit
that referenced
this pull request
Jun 25, 2024
ghstack-source-id: a4d25d25d63eec7c7a0f1bdfdb6a3a025af510ed Pull Request resolved: #128709
zou3519
approved these changes
Jun 26, 2024
…thon constant" When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang [ghstack-poisoned]
|
@pytorchbot merge |
Merge failedReason: This PR has internal changes and must be landed via Phabricator Details for Dev Infra teamRaised by workflow job |
…thon constant" When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames Differential Revision: [D59589709](https://our.internmc.facebook.com/intern/diff/D59589709) [ghstack-poisoned]
|
@ydwu4 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
…thon constant" When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames Differential Revision: [D59589709](https://our.internmc.facebook.com/intern/diff/D59589709) [ghstack-poisoned]
ydwu4
added a commit
that referenced
this pull request
Jul 10, 2024
ghstack-source-id: ad68a8daacb741710843b72bec69fd3ce0dc4247 Pull Request resolved: #128709
ydwu4
added a commit
that referenced
this pull request
Jul 10, 2024
…thon constant" Reland #128709. When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
|
replaced by #130493 |
ydwu4
added a commit
that referenced
this pull request
Jul 11, 2024
…hen pred is a python constant" Reland #128709. When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
ydwu4
added a commit
that referenced
this pull request
Jul 11, 2024
…thon constant" Reland #128709. When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
ydwu4
added a commit
that referenced
this pull request
Jul 11, 2024
…hen pred is a python constant" Reland #128709. When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
ydwu4
added a commit
that referenced
this pull request
Jul 11, 2024
…thon constant" Reland #128709. When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
pytorchmergebot
pushed a commit
that referenced
this pull request
Jul 12, 2024
…nt (#130493) Reland #128709. When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, Pull Request resolved: #130493 Approved by: https://github.com/BoyuanFeng
pytorchmergebot
pushed a commit
that referenced
this pull request
Jul 17, 2024
…, and prim::Exit (#129416) - Support raise exception. It's behavior matches non-strict export now, thanks to @ydwu4's [PR](#128709). - Support prim::Unitialized, prim::Enter, and prim::Exit Pull Request resolved: #129416 Approved by: https://github.com/angelayi
pytorchmergebot
pushed a commit
that referenced
this pull request
Jul 17, 2024
…, and prim::Exit (#129416) - Support raise exception. It's behavior matches non-strict export now, thanks to @ydwu4's [PR](#128709). - Support prim::Unitialized, prim::Enter, and prim::Exit Pull Request resolved: #129416 Approved by: https://github.com/angelayi
pytorchmergebot
pushed a commit
that referenced
this pull request
Jul 17, 2024
…, and prim::Exit (#129416) - Support raise exception. It's behavior matches non-strict export now, thanks to @ydwu4's [PR](#128709). - Support prim::Unitialized, prim::Enter, and prim::Exit Pull Request resolved: #129416 Approved by: https://github.com/angelayi
mlazos
pushed a commit
that referenced
this pull request
Jul 18, 2024
…, and prim::Exit (#129416) - Support raise exception. It's behavior matches non-strict export now, thanks to @ydwu4's [PR](#128709). - Support prim::Unitialized, prim::Enter, and prim::Exit Pull Request resolved: #129416 Approved by: https://github.com/angelayi
DiweiSun
pushed a commit
to DiweiSun/pytorch
that referenced
this pull request
Jul 22, 2024
…, and prim::Exit (pytorch#129416) - Support raise exception. It's behavior matches non-strict export now, thanks to @ydwu4's [PR](pytorch#128709). - Support prim::Unitialized, prim::Enter, and prim::Exit Pull Request resolved: pytorch#129416 Approved by: https://github.com/angelayi
DiweiSun
pushed a commit
to DiweiSun/pytorch
that referenced
this pull request
Jul 22, 2024
…, and prim::Exit (pytorch#129416) - Support raise exception. It's behavior matches non-strict export now, thanks to @ydwu4's [PR](pytorch#128709). - Support prim::Unitialized, prim::Enter, and prim::Exit Pull Request resolved: pytorch#129416 Approved by: https://github.com/angelayi
DiweiSun
pushed a commit
to DiweiSun/pytorch
that referenced
this pull request
Jul 22, 2024
…, and prim::Exit (pytorch#129416) - Support raise exception. It's behavior matches non-strict export now, thanks to @ydwu4's [PR](pytorch#128709). - Support prim::Unitialized, prim::Enter, and prim::Exit Pull Request resolved: pytorch#129416 Approved by: https://github.com/angelayi
xuhancn
pushed a commit
to xuhancn/pytorch
that referenced
this pull request
Jul 25, 2024
…nt (pytorch#128709) When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, Differential Revision: [D59589709](https://our.internmc.facebook.com/intern/diff/D59589709) Pull Request resolved: pytorch#128709 Approved by: https://github.com/zou3519
xuhancn
pushed a commit
to xuhancn/pytorch
that referenced
this pull request
Jul 25, 2024
…n constant (pytorch#128709)" This reverts commit fe3e687. Reverted pytorch#128709 on behalf of https://github.com/ydwu4 due to causing error on truck due to a land racing: https://hud.pytorch.org/pytorch/pytorch/commit/fe3e6878c4bb2a6001045c179fd7fa9838242558 ([comment](pytorch#128709 (comment)))
xuhancn
pushed a commit
to xuhancn/pytorch
that referenced
this pull request
Jul 25, 2024
…nt (pytorch#130493) Reland pytorch#128709. When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants. We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph. Test Plan: The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches, Pull Request resolved: pytorch#130493 Approved by: https://github.com/BoyuanFeng
xuhancn
pushed a commit
to xuhancn/pytorch
that referenced
this pull request
Jul 25, 2024
…, and prim::Exit (pytorch#129416) - Support raise exception. It's behavior matches non-strict export now, thanks to @ydwu4's [PR](pytorch#128709). - Support prim::Unitialized, prim::Enter, and prim::Exit Pull Request resolved: pytorch#129416 Approved by: https://github.com/angelayi
xuhancn
pushed a commit
to xuhancn/pytorch
that referenced
this pull request
Jul 25, 2024
…, and prim::Exit (pytorch#129416) - Support raise exception. It's behavior matches non-strict export now, thanks to @ydwu4's [PR](pytorch#128709). - Support prim::Unitialized, prim::Enter, and prim::Exit Pull Request resolved: pytorch#129416 Approved by: https://github.com/angelayi
xuhancn
pushed a commit
to xuhancn/pytorch
that referenced
this pull request
Jul 25, 2024
…, and prim::Exit (pytorch#129416) - Support raise exception. It's behavior matches non-strict export now, thanks to @ydwu4's [PR](pytorch#128709). - Support prim::Unitialized, prim::Enter, and prim::Exit Pull Request resolved: pytorch#129416 Approved by: https://github.com/angelayi
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stack from ghstack (oldest at bottom):
When the input predicate is a python constant, we specialize into one of the branches and warn users that torch.cond is not preserving the dynamism. The previous behavior is that we baked in True/False in the cond operator. This can be confusing. In this PR, we change it to be specializing into one of the branches when the inputs are constants.
We additionally change the naming of cond operator to default one without overriding its name. This allows better testing on de-serialized graph.
Test Plan:
The predicate in some existing tests is the result of a shape comparison. When no dynamic shape is involved, the predicate is a python bool. To fix them, we either change the predicate to be some data-dependent tensor or change the test to check cond is specialized as one of the branches,
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames
Differential Revision: D59589709