[dynamic shapes] guard_or_false for _reshape_view_helper, utils._infer_size for wildcard dims #150127
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/150127
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 2bca726 with merge base bc6c0bc ( BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
| ep = export(f, (torch.tensor(6),)) | ||
| ep.module()(torch.tensor(6)) | ||
| with self.assertRaisesRegex(RuntimeError, r"Runtime assertion failed for .* u0 .* 6"): | ||
| ep.module()(torch.tensor(5)) |
There was a problem hiding this comment.
exports fine now without check_is_size, we just assume it's >= 0 and it later specializes to 6.
test/export/test_export.py
Outdated
| y = torch.ones(5, 5) | ||
| ep = export(Foo(), (x, y)) | ||
| ep.module()(x, y) | ||
| ep.module()(torch.tensor(0), y) |
There was a problem hiding this comment.
test we can reshape [5+u0, 5] -> [5, u0 + 1] with a wildcard dim
test/export/test_export.py
Outdated
| error_type, | ||
| "Could not reshape a tensor with shape .*u0, u1.* as a tensor with shape .*u2, u3.*", | ||
| ): | ||
| export(Foo(), (xs,)) |
There was a problem hiding this comment.
very similar to the M_v0 test I deleted above, just (u0, u1) -> (u2, u3).
Adding the checks below work, and we specialize to the type of reshape that happens. Sadly more complicated relations are hard to torch._check into success (e.g. if the tensor was size [4, 6, 8, 3]). I would like to blame symbolic shapes axioms for that...
test/export/test_export.py
Outdated
| ): | ||
| export(N(), (t,), strict=strict) | ||
| export(N(), (t,)) |
There was a problem hiding this comment.
Here we're trying to reshape (u0, u1) -> (u0, u2). I thought the added torch._check in _reshape_view_helper that does u0*u1 == u0*u2 would help this succeed, but seems like symbolic shapes isn't strong enough for this (can't figure out that u1 == u2).
M_v0, M_v1, M_v2 all end up with the same error here.
test/export/test_export.py
Outdated
| torch._check(items[2] != -1) | ||
| # Could not guard on data-dependent expression u2 >= 0 | ||
| torch._check(items[2] >= 0) | ||
| # Could not guard on data-dependent expression Eq(u1, u2) | ||
| torch._check(items[2] == r.shape[1]) |
There was a problem hiding this comment.
now all you need is this check, the others are irrelevant
test/export/test_export.py
Outdated
| xs = torch.tensor([4, 4]) | ||
| ep = export(Foov3(), (xs,)) | ||
| ep.module()(xs) | ||
| ep.module()(torch.tensor([5, 5])) |
There was a problem hiding this comment.
This also works, because there's only one way to do this, and via the added torch._check(a.numel() == shape_numel) we assume in good faith the op is valid.
Maybe we shouldn't allow this though, it could be a typo from the user, and downstream this check could lead to complications that are hard to debug.
This change broke number of export tests, can we please be a bit more careful with the reviews
…k/oblivious_reshape_view
|
failure seems related but fix seems easy . |
|
@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 |
Sorry, something went wrong.
|
@pytorchbot revert -m "Caused TestDynamoTimed.test_dynamo_timed to fail on macOS, see https://github.com/pytorch/pytorch/actions/runs/14584536979/job/40908019050" -c nosignal |
|
@pytorchbot successfully started a revert job. Check the current status here. |
…ls._infer_size for wildcard dims (#150127)" This reverts commit a02eae8. Reverted #150127 on behalf of https://github.com/malfet due to Caused TestDynamoTimed.test_dynamo_timed to fail on macOS, see https://github.com/pytorch/pytorch/actions/runs/14584536979/job/40908019050 ([comment](#150127 (comment)))
|
@pianpwk your PR has been successfully reverted. |
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
|
Rebase failed due to Command Raised by https://github.com/pytorch/pytorch/actions/runs/14600107590 |
…k/oblivious_reshape_view
…k/oblivious_reshape_view
|
will land, CI looks good, and was told by @masnesral that previous failure was unrelated (flaky test) |
|
@pytorchmergebot 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 |
For reshape/view: removes fast paths for 0 elements, checking dimensions to skip. Modifies the loop accumulating input elements, to raise a UserError if we run out of dimensions, graph breaking for compile and erroring out for export.
For infer_size: assumes if user passes us an unbacked, it's probably not -1
Will think about changes in https://docs.google.com/document/d/1WYx6EZwVDXtBnWyrzoecgGWdiK0V3XZKftfpWwQ5i3E/edit?tab=t.0#heading=h.22k54zym11qp in a later PR