[subclasses] Handle dynamo inputs that are subclass views with (-1) in the view #128662
Conversation
…-1) in the view [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/128662
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (2 Unrelated Failures)As of commit 4e3e7c3 with merge base 4abecd7 ( FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
…iews with (-1) in the view" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang [ghstack-poisoned]
…iews with (-1) in the view" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang [ghstack-poisoned]
jbschlosser
left a comment
jbschlosser
left a comment
There was a problem hiding this comment.
LGTM as long as CI is clear, thanks for the fix!
| yield t.select(-1, 6), False | ||
| # https://github.com/pytorch/pytorch/issues/128649 | ||
| yield t[2:3, 5:9], dynamic | ||
| yield t.view(-1, 15), False |
There was a problem hiding this comment.
damn this was just missing :( AFAICT -1 is the only negative SymInt value you'd encounter in one of these views
torch/_subclasses/meta_utils.py
Outdated
| # e.g. if you .view(-1, ...) | ||
| positive = False | ||
|
|
||
| symbol = shape_env.create_symbol(s, sym_source, positive=positive) |
There was a problem hiding this comment.
cc @ezyang in case you have an opinion on whether positive=True or positive=None is better to use here when we don't have s < 0
…iews with (-1) in the view" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang [ghstack-poisoned]
davidberard98
changed the title
|
@pytorchbot merge |
Merge failedReason: This PR needs a If not, please add the To add a label, you can comment to pytorchbot, for example For more information, see Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge |
Merge startedYour 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 |
Merge failedReason: 1 jobs have failed, first few of them are: inductor / cuda12.1-py3.12-gcc9-sm86 / test (inductor, 1, 1, linux.g5.4xlarge.nvidia.gpu) Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
|
Successfully rebased |
|
@pytorchbot merge |
Merge startedYour 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 |
|
The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command |
|
@pytorchbot merge |
Merge startedYour 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 |
Stack from ghstack (oldest at bottom):
When handling an input to dynamo that's a view of a subclass, dynamo does some handling to reconstruct the view. Part of this is to construct symints for the input parameters to the view.
Previously, the code would just call
create_symbol()which by default specifies a positive symint (>= 0); this fails in the case where you have an aten::view that was called with a -1.Fix: just specify
positive=Nonewhen callingcreate_symbol(), to avoid restricting the symint to >= 0 or <= 0.cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang