Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/151095
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New Failure, 1 Pending, 14 Unrelated FailuresAs of commit 1c70eae with merge base 1f29190 ( NEW FAILURE - The following job has failed:
FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
BROKEN TRUNK - The following jobs 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. |
| self.assertEqual( | ||
| all_gathers[2].res_node.target, | ||
| torch.ops.aten.view.dtype, | ||
| torch.ops._c10d_functional.wait_tensor.default, |
There was a problem hiding this comment.
view.dtype is noop eliminated cc @weifengpy
There was a problem hiding this comment.
offline discussed with @weifengpy and the change makes sense
| b_inp = functools.partial(torch.empty, (1, 1, 8, 8), device=device) | ||
| m_inp = functools.partial(torch.empty, (2, 1, 1, 4), device=device) | ||
| # need 2d attn_mask to generate patterns with view op | ||
| m_inp_2d = functools.partial(torch.empty, (2, 4), device=device) |
There was a problem hiding this comment.
#113004 added _sfdp_pattern_15, which takes a 4d m_inp (shape: (2,1,1,4)) as attn_mask and calls (attn_mask == 0).view((bs, 1, 1, k_len)). After this pr, the view op will be noop eliminated so the search pattern will not have that view op. In _test_sdpa_rewriter_15, the graph still takes a 2d mask so the view op will not be noop eliminated. This difference prevents pattern match.
The change here still passes a 2d attn_mask so the view op is not noop eliminated in the search pattern.
| def view_default_noop(arg, size): | ||
| return arg.shape == tuple(size) | ||
|
|
There was a problem hiding this comment.
These should use statically_known_true here. We should not add a guard on arg[0] == size[0] if then arg[1] != size[1].
would be nice to have recursive api for this.
but just
return len(arg.shape) == len(size) and all(statically_known_true(a == b) for zip(arg.shape, size))
works for now.
cc @laithsakka
There was a problem hiding this comment.
apparently statically_known_true(sym_eq(ls1,ls2)) does this - thx u @laithsakka
| return arg.shape == size | ||
| @register_noop_decomp(aten.view.default) | ||
| def view_default_noop(arg, size): | ||
| return statically_known_true(sym_eq(arg.shape, tuple(size))) |
There was a problem hiding this comment.
arg.shape is a tuple but not a list. sym_eq returns False if one input is tuple and another input is list.
There was a problem hiding this comment.
Did this used to specialize on size?
|
@pytorchbot merge -f "skip unrelated export failure" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
This PR improves noop elimination.
### View Noop
```python
>>> torch.Size([1,2,3]) == [1,2,3]
False
>>> torch.Size([1,2,3]) == (1,2,3)
True
```
So we add `tuple(size)` in `view_noop`.
Example:
```python
import torch
@torch.compile()
def f(x):
batch_size = x.shape[0]
x = x.transpose(1, 2) # (batch_size, 2, 3)
x = x.reshape(batch_size, 2, 3) # noop
return x
x = torch.randn((2,3,2))
f(x)
x = torch.randn((4,3,2))
f(x)
```
Before:
After:
Pull Request resolved: pytorch#151095
Approved by: https://github.com/eellison
This PR improves noop elimination.
View Noop
So we add
tuple(size)inview_noop.Example:
Before:
After:
cc @H-Huang @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @chenyang78 @kadeng @muchulee8 @amjames @chauhang @aakhundov