Skip to content

Conversation

malfet
Copy link
Contributor

@malfet malfet commented Jan 3, 2024

To test it on MPS and CUDA devices
Also, move some float64 skip-tests for MPS to xfail, same as CPU tests for torch.half

Copy link

pytorch-bot bot commented Jan 3, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 46cf278 with merge base 3fe437b (image):
💚 Looks good so far! There are no failures yet. 💚

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

@malfet malfet added the topic: not user facing topic category label Jan 3, 2024
@malfet
Copy link
Contributor Author

malfet commented Jan 3, 2024

@pytorchbot drci

@malfet malfet requested a review from a team January 3, 2024 17:09
Copy link
Contributor

@mikaylagawarecki mikaylagawarecki left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the skip --> expected failure, will submit a follow up PR to fix some other skips in the file to expected failure

DecorateInfo(unittest.expectedFailure, "TestModule", "test_pickle", dtypes=[torch.float16], device_type='cpu'),
DecorateInfo(unittest.expectedFailure, "TestModule", "test_non_contiguous_tensors", dtypes=[torch.float16],
device_type='cpu'),
DecorateInfo(unittest.expectedFailure, "TestModule", "test_cpu_gpu_parity", dtypes=[torch.float16],
Copy link
Contributor

@mikaylagawarecki mikaylagawarecki Jan 3, 2024

Choose a reason for hiding this comment

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

For this case, this isn't that relevant since CPU does not have a float16 implementation for CrossEntropyLoss, but I wanted to note that test_cpu_gpu_parity is not tested for MPS as the test is decorated with @onlyCUDA,

@onlyCUDA
@with_tf32_off # Turn off TF32 to compute at full precision https://github.com/pytorch/pytorch/issues/86798
@toleranceOverride({torch.float32: tol(5e-2, 0),
torch.float64: tol(4e-4, 0)})
@modules(module_db)
def test_cpu_gpu_parity(self, device, dtype, module_info, training):

In general, do we want to enable this test for MPS as well? (I can do so in a followup if we should)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, let me submit a followup PR (as I guess it will require few more suppressions on MPS side)

@malfet malfet added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 3, 2024
@malfet
Copy link
Contributor Author

malfet commented Jan 3, 2024

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

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
Copy link
Collaborator

Merge failed

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

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@malfet
Copy link
Contributor Author

malfet commented Jan 4, 2024

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

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

@malfet malfet deleted the malfet/test-cross-entryopy-with-half branch January 4, 2024 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants