[MPS] Add `test/test_nn.py` to test suite #134184

hvaara · 2024-08-22T00:43:58Z

This PR increases test coverage by including the tests in test/test_nn.py in the test suite of MPS.

Some of the tests are decorated with @expectedFailureMPS for various reasons. Either that the op is not implemented, or that the outputs do not align. Those tests that contain differing results should be investigated further to rule out any live bugs.

$ python test/run_test.py --mps --verbose -k TestNN
Running test batch 'tests to run' cost 84.76 seconds

Ref #133520

cc @kulinseth @albanD @malfet @DenisVieriu97 @jhavukainen

pytorch-bot · 2024-08-22T00:44:01Z

🔗 Helpful Links

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

📄 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 c521e3c with merge base af66488 ():
💚 Looks good so far! There are no failures yet. 💚

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

hvaara · 2024-08-22T00:46:32Z

@pytorchbot label "module: mps" "topic: not user facing"

hvaara · 2024-08-22T00:52:22Z

This will likely fail, but I'll try anyways so maybe I can get some feedback for macOS 13 and 15.

@pytorchbot label "ciflow/mps"

pytorch-bot · 2024-08-22T00:52:29Z

Can't add following labels to PR: ciflow/mps. Please ping one of the reviewers for help.

hvaara · 2024-08-22T00:54:17Z

Requesting approval for this change from @kulinseth or @malfet as code owners for MPS.

skotapati · 2024-08-22T19:22:37Z

@hvaara thanks for opening this pr. I went ahead and launched your CI run so we can see what happens, but just fyi that I may open a separate pr to add test_nn and a few others to the mps flow

hvaara · 2024-08-22T19:30:49Z

Thanks @skotapati!

but just fyi that I may open a separate pr to add test_nn and a few others to the mps flow

I don't understand. Do you mean you want me to split this up, or did you mean you're already working on something similar? Do you have a PR?

Attempting to add MPS ciflow again.

@pytorchbot label "ciflow/mps"

hvaara · 2024-08-22T21:00:15Z

Tests pass on macOS 14 and 15.

Detected failure on macOS 13:TestNNDeviceTypeMPS.test_conv_empty_input_mps_bfloat16 due to TypeError: MPS BFloat16 is only supported on MacOS 14 or newer. Skipping this test on macOS 13.

@pytorchbot label keep-going

hvaara · 2024-08-22T22:23:46Z

I think that was the last of it in MPS - adding trunk.

@pytorchbot label ciflow/trunk

pytorch-bot · 2024-08-22T22:23:53Z

Can't add following labels to PR: ciflow/trunk. Please ping one of the reviewers for help.

hvaara · 2024-08-23T03:31:23Z

I think the test runner died before starting the build step, but I see no way of restarting the tests. Will force push

…acOS versions

hvaara · 2024-08-23T16:32:35Z

Had to force-push again. There's been several incidents with Actions lately, but the tests are running again now.

albanD

SGTM
How do you plan to track these disables to work on fixing them?

pytorchmergebot · 2024-08-26T22:47:44Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Lint / lintrunner-noclang / linux-job

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

malfet · 2024-08-26T23:08:32Z

@hvaara you need to fix lint before this change can go in (and I can't do it for you, as you do not accept changes from contributors on your fork of PyTorch)

hvaara · 2024-08-26T23:20:50Z

Thanks for looking, @malfet. Fixed it and ran make lint locally this time to verify resolution.

The following option is enabled

am I missing something?

hvaara · 2024-08-26T23:26:23Z

though in the long run we probably don't need to add those to MPS shard, as they'll be picked up by trunk

How will they be picked up by trunk? Is this something that's planned for the future?

malfet · 2024-08-26T23:46:44Z

@pytorchbot merge -f "Lint is green and CI was green during previous iteration"

pytorchmergebot · 2024-08-26T23:48:14Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

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

malfet · 2024-08-27T00:38:40Z

though in the long run we probably don't need to add those to MPS shard, as they'll be picked up by trunk

How will they be picked up by trunk? Is this something that's planned for the future?

test_nn.py is running as part of trunk workflow, and it should cover MPS as well after your change, see https://github.com/pytorch/pytorch/actions/runs/10568390813/job/29279246604

hvaara · 2024-08-27T00:49:29Z

though in the long run we probably don't need to add those to MPS shard, as they'll be picked up by trunk

How will they be picked up by trunk? Is this something that's planned for the future?

test_nn.py is running as part of trunk workflow, and it should cover MPS as well after your change, see https://github.com/pytorch/pytorch/actions/runs/10568390813/job/29279246604

Do you mean it's sufficient to set allow_mps=True in https://github.com/hvaara/pytorch/blob/c521e3c4df81ea587055cf27f4aae00f3ef269a8/test/test_nn.py#L12997 and not include test_nn in https://github.com/hvaara/pytorch/blob/c521e3c4df81ea587055cf27f4aae00f3ef269a8/test/run_test.py#L1425 ?

hvaara · 2024-09-06T14:19:22Z

git show 1565940114aa271749e6395948ae81ff259868b4 | grep -e '^+.*issues/' | sed -e 's#.*issues/##' | sort -h | uniq

xref:

Updating the comments with references to better places for context now that the bugs have been identified. xref #135442 #135447 #134184 Pull Request resolved: #135448 Approved by: https://github.com/ezyang

This PR increases test coverage by including the tests in `test/test_nn.py` in the test suite of MPS. Some of the tests are decorated with `@expectedFailureMPS` for various reasons. Either that the op is not implemented, or that the outputs do not align. Those tests that contain differing results should be investigated further to rule out any live bugs. ```bash $ python test/run_test.py --mps --verbose -k TestNN Running test batch 'tests to run' cost 84.76 seconds ``` Ref pytorch#133520 Pull Request resolved: pytorch#134184 Approved by: https://github.com/albanD, https://github.com/malfet

Updating the comments with references to better places for context now that the bugs have been identified. xref pytorch#135442 pytorch#135447 pytorch#134184 Pull Request resolved: pytorch#135448 Approved by: https://github.com/ezyang

hvaara requested review from mruberry and a team as code owners August 22, 2024 00:43

pytorch-bot bot added the release notes: nn release notes category label Aug 22, 2024

pytorchbot added the open source label Aug 22, 2024

pytorch-bot bot added module: mps Related to Apple Metal Performance Shaders framework topic: not user facing topic category labels Aug 22, 2024

hvaara added 2 commits August 22, 2024 03:16

[MPS] Add test_nn to test suite

ed9b19e

Fix lint errors

c2eff44

hvaara force-pushed the test-nn-include branch from 46153a2 to c2eff44 Compare August 22, 2024 01:18

pytorch-bot bot added the ciflow/mps Run MPS tests (subset of trunk) label Aug 22, 2024

Skip test with dtype bfloat16 for macOS 13

d9a16ce

pytorch-bot bot added the keep-going Don't stop on first failure, keep running tests until the end label Aug 22, 2024

Fix two additional bfloat16 failures on macOS 13

ce6c003

hvaara mentioned this pull request Aug 23, 2024

[pytorchbot] retest command pytorch/test-infra#473

Open

hvaara force-pushed the test-nn-include branch from a46ae8c to b149887 Compare August 23, 2024 03:32

Skip test_transformerencoderlayer due to inconsistent result across m…

c19005d

…acOS versions

hvaara force-pushed the test-nn-include branch from b149887 to c19005d Compare August 23, 2024 16:11

albanD approved these changes Aug 23, 2024

View reviewed changes

pytorchmergebot removed the merging label Aug 26, 2024

Fix lint

c521e3c

pytorchmergebot added the merging label Aug 26, 2024

pytorchmergebot added the Merged label Aug 26, 2024

pytorchmergebot closed this in 1565940 Aug 26, 2024

pytorchmergebot removed the merging label Aug 26, 2024

hvaara deleted the test-nn-include branch August 27, 2024 00:49

hvaara mentioned this pull request Aug 27, 2024

[MPS] BatchNorm2D produces incorrect results for column first tensors #134580

Open

This was referenced Sep 8, 2024

[MPS] Possible persistent infinite loop in nn.ReplicationPad1d #135442

Open

[MPS] Correctness issue in backward pass of nn.ReplicationPad1d and nn.ReplicationPad2d #135447

Open

[MPS] Update decorator comments with issue ref #135448

Closed

hvaara mentioned this pull request Sep 9, 2024

[MPS] Allow nan mean reduction in nll_loss #135434

Closed

hvaara mentioned this pull request Sep 27, 2024

[MPS] Incorrect result from batch norm with sliced inputs #133520

Closed

[MPS] Add test/test_nn.py to test suite #134184

[MPS] Add test/test_nn.py to test suite #134184

Uh oh!

Conversation

hvaara commented Aug 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 22, 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/134184

✅ No Failures

Uh oh!

hvaara commented Aug 22, 2024

Uh oh!

hvaara commented Aug 22, 2024

Uh oh!

pytorch-bot bot commented Aug 22, 2024

Uh oh!

hvaara commented Aug 22, 2024

Uh oh!

skotapati commented Aug 22, 2024

Uh oh!

hvaara commented Aug 22, 2024

Uh oh!

hvaara commented Aug 22, 2024

Uh oh!

hvaara commented Aug 22, 2024

Uh oh!

pytorch-bot bot commented Aug 22, 2024

Uh oh!

hvaara commented Aug 23, 2024

Uh oh!

hvaara commented Aug 23, 2024

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

pytorchmergebot commented Aug 26, 2024

Merge failed

Uh oh!

malfet commented Aug 26, 2024

Uh oh!

hvaara commented Aug 26, 2024

Uh oh!

hvaara commented Aug 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

malfet commented Aug 26, 2024

Uh oh!

pytorchmergebot commented Aug 26, 2024

Merge started

Uh oh!

malfet commented Aug 27, 2024

Uh oh!

hvaara commented Aug 27, 2024

Uh oh!

hvaara commented Sep 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

[MPS] Add `test/test_nn.py` to test suite #134184

[MPS] Add `test/test_nn.py` to test suite #134184

hvaara commented Aug 22, 2024 •

edited

Loading

pytorch-bot bot commented Aug 22, 2024 •

edited

Loading

hvaara commented Aug 26, 2024 •

edited

Loading

hvaara commented Sep 6, 2024 •

edited

Loading