Skip to content

Conversation

@kulinseth
Copy link
Collaborator

  • Fix view op slicing for 2nd dim in case of 0 offset

* Fix view op slicing for 2nd dim in case of 0 offset
@pytorch-bot
Copy link

pytorch-bot bot commented Feb 23, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/95381

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 1d57051:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added ciflow/mps Run MPS tests (subset of trunk) release notes: mps Release notes category labels Feb 23, 2023
Copy link
Collaborator

@razarmehr razarmehr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@razarmehr
Copy link
Collaborator

@pytorchbot merge -f "MPS and Lint are green"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Feb 23, 2023
)

* Fix view op slicing for 2nd dim in case of 0 offset

Pull Request resolved: pytorch#95381
Approved by: https://github.com/razarmehr
This was referenced Feb 23, 2023
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Feb 23, 2023
)

* Fix view op slicing for 2nd dim in case of 0 offset

Pull Request resolved: pytorch#95381
Approved by: https://github.com/razarmehr
atalman pushed a commit that referenced this pull request Feb 24, 2023
* [MPS] Fix the uint8 type issue with View ops kernels (#95145)

This should fix the problem in Resnet model with image artifacts due to saturation on int8 type and also the incorrect class recognition reported in #86954.

Fixes #86954

Pull Request resolved: #95145
Approved by: https://github.com/kulinseth, https://github.com/DenisVieriu97

* [MPS] Fix tensor with non-zero storage offset graph gathering (#91071)

Previously, the "can slice" flag in Placeholder constructor in `OperationUtils.mm` is conditioned on whether the numbers of dimensions of base shape and view shape are the same. This doesn't consider the situation that a view tensor could be the base tensor's sliced and then unsqueezed version, resulting in different num of dims.

For example, if we want to stack `y_mps` and `x_mps` on the last dim:
```
t_mps = torch.tensor([1, 2, 3, 4], device="mps")
x_mps = t_mps[2:]  # [3, 4]
y_mps = t_mps[:2]  # [1, 2]

res_mps = torch.stack((y_mps, x_mps), dim=-1)
```

the kernel will unsqueeze both of them on the last dim and then concatenate them, which is equivalent to:

```
res_mps = torch.cat((y_mps.unsqueeze(-1), x_mps.unsqueeze(-1)), dim=-1)
```

`x_mps.unsqueeze(-1)` is an unsqueezed and contiguous tensor with a storage offset, this kind of tensors should be sliceable without cloning its storage.

Fixes #87856
Fixes #91065

Pull Request resolved: #91071
Approved by: https://github.com/kulinseth

* [MPS] Fix fill_ where input tensor has a storage offset (#95113)

Fixes #94390

Apart from fixing the issue above, this PR also fixes a bug that when an input tensor can be sliced, a sliced array view is created. This array view seems to be not writable or have a different storage from the original tensor, causing incorrect results with the in-place `fill`.
Pull Request resolved: #95113
Approved by: https://github.com/kulinseth

* [MPS] Fix view op slicing for 2nd dim in case of 0 offset (#95381)

* Fix view op slicing for 2nd dim in case of 0 offset

Pull Request resolved: #95381
Approved by: https://github.com/razarmehr

---------

Co-authored-by: Ramin Azarmehr <razarmehr@apple.com>
Co-authored-by: Li-Huai (Allan) Lin <qqaatw@gmail.com>
Co-authored-by: Denis Vieriu <104024078+DenisVieriu97@users.noreply.github.com>
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Feb 25, 2023
* Fix view op slicing for 2nd dim in case of 0 offset

Pull Request resolved: pytorch/pytorch#95381
Approved by: https://github.com/razarmehr
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Feb 25, 2023
* Fix view op slicing for 2nd dim in case of 0 offset

Pull Request resolved: pytorch/pytorch#95381
Approved by: https://github.com/razarmehr
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Mar 5, 2023
* Fix view op slicing for 2nd dim in case of 0 offset

Pull Request resolved: pytorch/pytorch#95381
Approved by: https://github.com/razarmehr
pruthvistony added a commit to ROCm/pytorch that referenced this pull request May 2, 2023
pruthvistony pushed a commit to ROCm/pytorch that referenced this pull request May 3, 2023
* [MPS] Fix the uint8 type issue with View ops kernels (pytorch#95145)

This should fix the problem in Resnet model with image artifacts due to saturation on int8 type and also the incorrect class recognition reported in pytorch#86954.

Fixes pytorch#86954

Pull Request resolved: pytorch#95145
Approved by: https://github.com/kulinseth, https://github.com/DenisVieriu97

* [MPS] Fix tensor with non-zero storage offset graph gathering (pytorch#91071)

Previously, the "can slice" flag in Placeholder constructor in `OperationUtils.mm` is conditioned on whether the numbers of dimensions of base shape and view shape are the same. This doesn't consider the situation that a view tensor could be the base tensor's sliced and then unsqueezed version, resulting in different num of dims.

For example, if we want to stack `y_mps` and `x_mps` on the last dim:
```
t_mps = torch.tensor([1, 2, 3, 4], device="mps")
x_mps = t_mps[2:]  # [3, 4]
y_mps = t_mps[:2]  # [1, 2]

res_mps = torch.stack((y_mps, x_mps), dim=-1)
```

the kernel will unsqueeze both of them on the last dim and then concatenate them, which is equivalent to:

```
res_mps = torch.cat((y_mps.unsqueeze(-1), x_mps.unsqueeze(-1)), dim=-1)
```

`x_mps.unsqueeze(-1)` is an unsqueezed and contiguous tensor with a storage offset, this kind of tensors should be sliceable without cloning its storage.

Fixes pytorch#87856
Fixes pytorch#91065

Pull Request resolved: pytorch#91071
Approved by: https://github.com/kulinseth

* [MPS] Fix fill_ where input tensor has a storage offset (pytorch#95113)

Fixes pytorch#94390

Apart from fixing the issue above, this PR also fixes a bug that when an input tensor can be sliced, a sliced array view is created. This array view seems to be not writable or have a different storage from the original tensor, causing incorrect results with the in-place `fill`.
Pull Request resolved: pytorch#95113
Approved by: https://github.com/kulinseth

* [MPS] Fix view op slicing for 2nd dim in case of 0 offset (pytorch#95381)

* Fix view op slicing for 2nd dim in case of 0 offset

Pull Request resolved: pytorch#95381
Approved by: https://github.com/razarmehr

---------

Co-authored-by: Ramin Azarmehr <razarmehr@apple.com>
Co-authored-by: Li-Huai (Allan) Lin <qqaatw@gmail.com>
Co-authored-by: Denis Vieriu <104024078+DenisVieriu97@users.noreply.github.com>
jhavukainen pushed a commit to kulinseth/pytorch that referenced this pull request Mar 15, 2024
)

* Fix view op slicing for 2nd dim in case of 0 offset

Pull Request resolved: pytorch#95381
Approved by: https://github.com/razarmehr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/mps Run MPS tests (subset of trunk) Merged open source release notes: mps Release notes category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants