Skip to content

[MPS][TYPE_PROMOTION] Fix Clamp #133260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

pytorchbot
Copy link
Collaborator

Stack from ghstack (oldest at bottom):

Summary:

  1. Fixed tensor.clamp produces wrong values on MPS #130201 by adding type promotion.
  2. Added proper tests.
  3. Found torch's type promotion is different from numpy as follows:
import torch
import numpy as np
np.clip(np.array([1], dtype=np.float32), np.array([1], dtype=np.int32), None).dtype  # dtype('float64')
torch.clamp(torch.tensor([1], dtype=torch.float32), torch.tensor([1], dtype=torch.int32)).dtype  # torch.float32

Not sure the proper way to handle it, it causes numpy ref tests to fail.
Reason here, so think I'm gonna xfail it:

pytorch/test/test_ops.py

Lines 260 to 264 in 3c1cf03

# Tests that the function and its (ndarray-accepting) reference produce the same
# values on the tensors from sample_inputs func for the corresponding op.
# This test runs in double and complex double precision because
# NumPy does computation internally using double precision for many functions
# resulting in possible equality check failures.

Summary:
1. Fixed #130201 by adding type promotion.
2. Added proper tests.
3. Found torch's type promotion is different from numpy as follows:

```python
import torch
import numpy as np
np.clip(np.array([1], dtype=np.float32), np.array([1], dtype=np.int32), None).dtype  # dtype('float64')
torch.clamp(torch.tensor([1], dtype=torch.float32), torch.tensor([1], dtype=torch.int32)).dtype  # torch.float32
```

~Not sure the proper way to handle it, it causes numpy ref tests to fail.~
Reason here, so think I'm gonna xfail it:
https://github.com/pytorch/pytorch/blob/3c1cf03fde145bdbe1f5ffb81765d076c10b4c04/test/test_ops.py#L260-L264

Pull Request resolved: #130226
Approved by: https://github.com/malfet

(cherry picked from commit 99967e1)
Copy link

pytorch-bot bot commented Aug 12, 2024

🔗 Helpful Links

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

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

❌ 3 New Failures, 1 Cancelled Job

As of commit 3eb6294 with merge base b66e3f0 (image):

NEW FAILURES - The following jobs have failed:

CANCELLED JOB - The following job was cancelled. Please retry:

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 Aug 12, 2024
@malfet malfet merged commit 26735e7 into release/2.4 Aug 13, 2024
104 of 108 checks passed
pruthvistony pushed a commit to ROCm/pytorch that referenced this pull request Aug 15, 2024
[MPS][TYPE_PROMOTION] Fix Clamp (pytorch#130226)

Summary:
1. Fixed pytorch#130201 by adding type promotion.
2. Added proper tests.
3. Found torch's type promotion is different from numpy as follows:

```python
import torch
import numpy as np
np.clip(np.array([1], dtype=np.float32), np.array([1], dtype=np.int32), None).dtype  # dtype('float64')
torch.clamp(torch.tensor([1], dtype=torch.float32), torch.tensor([1], dtype=torch.int32)).dtype  # torch.float32
```

~Not sure the proper way to handle it, it causes numpy ref tests to fail.~
Reason here, so think I'm gonna xfail it:
https://github.com/pytorch/pytorch/blob/3c1cf03fde145bdbe1f5ffb81765d076c10b4c04/test/test_ops.py#L260-L264

Pull Request resolved: pytorch#130226
Approved by: https://github.com/malfet

(cherry picked from commit 99967e1)

Co-authored-by: Li-Huai (Allan) Lin <[email protected]>
@github-actions github-actions bot deleted the cherry-pick-130226-by-pytorch_bot_bot_ branch September 17, 2024 01:55
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) open source release notes: mps Release notes category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants