[BE] Add dtypesIfMPS to ModuleInfo enabling float16 tests for MPS and remove all skipIfMPS for float64 #119039

mikaylagawarecki · 2024-02-02T17:22:40Z

Right now, ModuleInfo.dtypes defaults to torch.testing._internal.common_dtype.floating_types(), almost no ModuleInfos override this (so only float32 and float64 are tested).

This is the first step to clean up/improve dtype testing for ModuleInfos and fix #116626.

Follow up PRs will updates dtypes= (and perhaps dtypesIf{Device} (if it makes sense)) for each ModuleInfo

Stack from ghstack (oldest at bottom):

-> [BE] Add dtypesIfMPS to ModuleInfo enabling float16 tests for MPS and remove all skipIfMPS for float64 #119039

[ghstack-poisoned]

pytorch-bot · 2024-02-02T17:22:44Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

✅ No Failures

As of commit f242fa5 with merge base 0a41ac3 ():
💚 Looks good so far! There are no failures yet. 💚

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

torch/testing/_internal/common_modules.py

…S for float64" Right now, ModuleInfo.dtypes defaults to `torch.testing._internal.common_dtype.floating_types()`, almost no ModuleInfos override this (so only `float32` and `float64` are tested). This is the first step to clean up dtype testing for `ModuleInfos` and fix #116626. Follow up PRs will updates `dtypes=` (and perhaps `dtypesIf{Device}` for each `ModuleInfo` [ghstack-poisoned]

…S for float64" Right now, `ModuleInfo.dtypes` defaults to `torch.testing._internal.common_dtype.floating_types()`, almost no ModuleInfos override this (so only `float32` and `float64` are tested). This is the first step to clean up/improve dtype testing for `ModuleInfos` and fix #116626. Follow up PRs will updates `dtypes=` (and perhaps `dtypesIf{Device}` (if it makes sense) for each `ModuleInfo` [ghstack-poisoned]

…S for float64" Right now, `ModuleInfo.dtypes` defaults to `torch.testing._internal.common_dtype.floating_types()`, almost no ModuleInfos override this (so only `float32` and `float64` are tested). This is the first step to clean up/improve dtype testing for `ModuleInfos` and fix #116626. Follow up PRs will updates `dtypes=` (and perhaps `dtypesIf{Device}` (if it makes sense)) for each `ModuleInfo` [ghstack-poisoned]

ghstack-source-id: 2ed3692 Pull Request resolved: #119039

janeyx99 · 2024-02-05T16:12:14Z

torch/testing/_internal/common_modules.py

                 skips=(),  # Indicates which tests to skip
                 decorators=None,  # Additional decorators to apply to generated tests
                 dtypes=floating_types(),  # dtypes this function is expected to work with
+                 dtypesIfMPS=(torch.float16, torch.float32,),  # dtypes this function is expected to work with on MPS


I am curious how we would scale this in the future when there may be more accelerators. I'd guess we'd have an API somewhat like accelerator.get_supported_dtypes() and the dtypes field here could be a dynamic function that gets the current accelerator and returns the relevant dtypes, similar to your supported_dtypes function.

This then brings me to think "hey, maybe in the future there would be an accelerator-aware supported_floating_types() vs what's used in line 213". Anyway, this discussion is likely outside the scope of this PR. cc'ing @albanD in case he has thoughts

Great point! I was thinking about this too :)

I don't think we will have BC issues changing the dtype logic in the test infra. Updating dtypes to account for the current accelerator in the future definitely sounds reasonable to me

…and remove all skipIfMPS for float64" Right now, `ModuleInfo.dtypes` defaults to `torch.testing._internal.common_dtype.floating_types()`, almost no ModuleInfos override this (so only `float32` and `float64` are tested). This is the first step to clean up/improve dtype testing for `ModuleInfos` and fix #116626. Follow up PRs will updates `dtypes=` (and perhaps `dtypesIf{Device}` (if it makes sense)) for each `ModuleInfo` [ghstack-poisoned]

ghstack-source-id: 2986114 Pull Request resolved: #119039

…for MPS and remove all skipIfMPS for float64" Right now, `ModuleInfo.dtypes` defaults to `torch.testing._internal.common_dtype.floating_types()`, almost no ModuleInfos override this (so only `float32` and `float64` are tested). This is the first step to clean up/improve dtype testing for `ModuleInfos` and fix #116626. Follow up PRs will updates `dtypes=` (and perhaps `dtypesIf{Device}` (if it makes sense)) for each `ModuleInfo` [ghstack-poisoned]

mikaylagawarecki · 2024-02-07T23:27:39Z

@pytorchbot merge

No MPS failures on PR

pytorchmergebot · 2024-02-07T23:29:31Z

Merge started

Your 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

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2024-02-07T23:30:05Z

Merge failed

Reason: Command git -C /home/runner/work/pytorch/pytorch cherry-pick -x b7171a9b0d7ab6da3eef10e8253d96b5d2ad3358 returned non-zero exit code 1

Auto-merging torch/testing/_internal/common_modules.py
CONFLICT (content): Merge conflict in torch/testing/_internal/common_modules.py
error: could not apply b7171a9b0d7... Add dtypesIfMPS to ModuleInfo and remove all float64 skips
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

Details for Dev Infra team

Raised by workflow job

…for MPS and remove all skipIfMPS for float64" Right now, `ModuleInfo.dtypes` defaults to `torch.testing._internal.common_dtype.floating_types()`, almost no ModuleInfos override this (so only `float32` and `float64` are tested). This is the first step to clean up/improve dtype testing for `ModuleInfos` and fix #116626. Follow up PRs will updates `dtypes=` (and perhaps `dtypesIf{Device}` (if it makes sense)) for each `ModuleInfo` [ghstack-poisoned]

ghstack-source-id: fab1dba Pull Request resolved: #119039

…for MPS and remove all skipIfMPS for float64" Right now, `ModuleInfo.dtypes` defaults to `torch.testing._internal.common_dtype.floating_types()`, almost no ModuleInfos override this (so only `float32` and `float64` are tested). This is the first step to clean up/improve dtype testing for `ModuleInfos` and fix #116626. Follow up PRs will updates `dtypes=` (and perhaps `dtypesIf{Device}` (if it makes sense)) for each `ModuleInfo` [ghstack-poisoned]

ghstack-source-id: feb5e0b Pull Request resolved: #119039

mikaylagawarecki · 2024-02-08T17:31:08Z

@pytorchbot rebase -s

pytorchmergebot · 2024-02-08T17:32:54Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

pytorchmergebot · 2024-02-08T17:33:03Z

Tried to rebase and push PR #119039, but it was already up to date. Try rebasing against main by issuing:
@pytorchbot rebase -b main

mikaylagawarecki · 2024-02-08T19:50:14Z

@pytorchbot merge

pytorchmergebot · 2024-02-08T19:52:09Z

Merge started

Your 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

Advanced Debugging

Check the merge workflow status
here

… remove all skipIfMPS for float64 (#119039) Right now, `ModuleInfo.dtypes` defaults to `torch.testing._internal.common_dtype.floating_types()`, almost no ModuleInfos override this (so only `float32` and `float64` are tested). This is the first step to clean up/improve dtype testing for `ModuleInfos` and fix #116626. Follow up PRs will updates `dtypes=` (and perhaps `dtypesIf{Device}` (if it makes sense)) for each `ModuleInfo` Pull Request resolved: #119039 Approved by: https://github.com/janeyx99

… MPS and remove all skipIfMPS for float64 (#119039)" This reverts commit 04d52d5. Reverted #119039 on behalf of https://github.com/huydhn due to Sorry for reverting your change but it is failing MPS test in trunk https://hud.pytorch.org/pytorch/pytorch/commit/04d52d5399ad4abb8af9e8405be79e2a7f8b4c7a, may be a landrace ([comment](#119039 (comment)))

… remove all skipIfMPS for float64 (#119039) Right now, `ModuleInfo.dtypes` defaults to `torch.testing._internal.common_dtype.floating_types()`, almost no ModuleInfos override this (so only `float32` and `float64` are tested). This is the first step to clean up/improve dtype testing for `ModuleInfos` and fix #116626. Follow up PRs will updates `dtypes=` (and perhaps `dtypesIf{Device}` (if it makes sense)) for each `ModuleInfo` Pull Request resolved: #119039 Approved by: https://github.com/janeyx99

… MPS and remove all skipIfMPS for float64 (#119039)" This reverts commit 04d52d5. Reverted #119039 on behalf of https://github.com/huydhn due to Sorry for reverting your change but it is failing MPS test in trunk https://hud.pytorch.org/pytorch/pytorch/commit/04d52d5399ad4abb8af9e8405be79e2a7f8b4c7a, may be a landrace ([comment](#119039 (comment)))

… remove all skipIfMPS for float64 (#119039) Right now, `ModuleInfo.dtypes` defaults to `torch.testing._internal.common_dtype.floating_types()`, almost no ModuleInfos override this (so only `float32` and `float64` are tested). This is the first step to clean up/improve dtype testing for `ModuleInfos` and fix #116626. Follow up PRs will updates `dtypes=` (and perhaps `dtypesIf{Device}` (if it makes sense)) for each `ModuleInfo` Pull Request resolved: #119039 Approved by: https://github.com/janeyx99

Add dtypesIfMPS to ModuleInfo and remove all float64 skips

d73525e

[ghstack-poisoned]

mikaylagawarecki requested review from albanD and janeyx99 February 2, 2024 17:23

mikaylagawarecki changed the title ~~Add dtypesIfMPS to ModuleInfo and remove all float64 skips~~ Add dtypesIfMPS to ModuleInfo and remove all skipIfMPS for float64 Feb 2, 2024

mikaylagawarecki changed the title ~~Add dtypesIfMPS to ModuleInfo and remove all skipIfMPS for float64~~ [BE] Add dtypesIfMPS to ModuleInfo and remove all skipIfMPS for float64 Feb 2, 2024

mikaylagawarecki added ciflow/trunk Trigger trunk jobs on your pull request topic: not user facing topic category labels Feb 2, 2024

janeyx99 reviewed Feb 2, 2024

View reviewed changes

torch/testing/_internal/common_modules.py Outdated Show resolved Hide resolved

albanD requested review from janeyx99 and malfet and removed request for albanD February 2, 2024 17:27

malfet added the ciflow/mps Run MPS tests (subset of trunk) label Feb 2, 2024

mikaylagawarecki marked this pull request as draft February 2, 2024 20:03

mikaylagawarecki added keep-going Don't stop on first failure, keep running tests until the end and removed ciflow/trunk Trigger trunk jobs on your pull request labels Feb 2, 2024

mikaylagawarecki mentioned this pull request Feb 3, 2024

[MPS] Tracking issue for ModuleInfo failures when enabling testing for torch.float16 #119108

Open

39 tasks

mikaylagawarecki added a commit that referenced this pull request Feb 3, 2024

Add dtypesIfMPS to ModuleInfo and remove all float64 skips

5656b49

ghstack-source-id: 2ed3692 Pull Request resolved: #119039

mikaylagawarecki changed the title ~~[BE] Add dtypesIfMPS to ModuleInfo and remove all skipIfMPS for float64~~ [BE] Add dtypesIfMPS to ModuleInfo enabling float16 tests and remove all skipIfMPS for float64 Feb 3, 2024

janeyx99 reviewed Feb 5, 2024

View reviewed changes

mikaylagawarecki added a commit that referenced this pull request Feb 5, 2024

Add dtypesIfMPS to ModuleInfo and remove all float64 skips

bad543c

ghstack-source-id: 2986114 Pull Request resolved: #119039

pytorchmergebot added the Reverted label Feb 6, 2024

pytorchmergebot reopened this Feb 6, 2024

pytorchmergebot added the merging label Feb 7, 2024

pytorchmergebot removed the merging label Feb 7, 2024

mikaylagawarecki added a commit that referenced this pull request Feb 8, 2024

Add dtypesIfMPS to ModuleInfo and remove all float64 skips

acf3ac1

ghstack-source-id: fab1dba Pull Request resolved: #119039

mikaylagawarecki added a commit that referenced this pull request Feb 8, 2024

Add dtypesIfMPS to ModuleInfo and remove all float64 skips

2ee3800

ghstack-source-id: feb5e0b Pull Request resolved: #119039

pytorchmergebot added the merging label Feb 8, 2024

pytorchmergebot closed this in db1a4dc Feb 8, 2024

pytorchmergebot removed the merging label Feb 8, 2024

github-actions bot deleted the gh/mikaylagawarecki/184/head branch March 10, 2024 01:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[BE] Add dtypesIfMPS to ModuleInfo enabling float16 tests for MPS and remove all skipIfMPS for float64 #119039

[BE] Add dtypesIfMPS to ModuleInfo enabling float16 tests for MPS and remove all skipIfMPS for float64 #119039

Uh oh!

mikaylagawarecki commented Feb 2, 2024 •

edited

Loading

Uh oh!

pytorch-bot bot commented Feb 2, 2024 •

edited

Loading

Uh oh!

Uh oh!

janeyx99 Feb 5, 2024

Uh oh!

mikaylagawarecki Feb 5, 2024

Uh oh!

mikaylagawarecki commented Feb 7, 2024

Uh oh!

pytorchmergebot commented Feb 7, 2024

Uh oh!

pytorchmergebot commented Feb 7, 2024

Uh oh!

mikaylagawarecki commented Feb 8, 2024

Uh oh!

pytorchmergebot commented Feb 8, 2024

Uh oh!

pytorchmergebot commented Feb 8, 2024

Uh oh!

mikaylagawarecki commented Feb 8, 2024

Uh oh!

pytorchmergebot commented Feb 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[BE] Add dtypesIfMPS to ModuleInfo enabling float16 tests for MPS and remove all skipIfMPS for float64 #119039

[BE] Add dtypesIfMPS to ModuleInfo enabling float16 tests for MPS and remove all skipIfMPS for float64 #119039

Uh oh!

Conversation

mikaylagawarecki commented Feb 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Feb 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ No Failures

Uh oh!

Uh oh!

janeyx99 Feb 5, 2024

Choose a reason for hiding this comment

Uh oh!

mikaylagawarecki Feb 5, 2024

Choose a reason for hiding this comment

Uh oh!

mikaylagawarecki commented Feb 7, 2024

Uh oh!

pytorchmergebot commented Feb 7, 2024

Merge started

Uh oh!

pytorchmergebot commented Feb 7, 2024

Merge failed

Uh oh!

mikaylagawarecki commented Feb 8, 2024

Uh oh!

pytorchmergebot commented Feb 8, 2024

Uh oh!

pytorchmergebot commented Feb 8, 2024

Uh oh!

mikaylagawarecki commented Feb 8, 2024

Uh oh!

pytorchmergebot commented Feb 8, 2024

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mikaylagawarecki commented Feb 2, 2024 •

edited

Loading

pytorch-bot bot commented Feb 2, 2024 •

edited

Loading