Update test_c10d_object_collectives.py with DistributedTestBase class #145056

amathewc · 2025-01-17T07:23:04Z

MOTIVATION

To generalize distributed test cases for non-CUDA devices, we are leveraging the DistributedTestBase class introduced in PR #138216. This new class is derived from MultiProcessTestCase and abstracts the creation/deletion of process groups and other functionality for specific devices. In this PR, we extend the scope of these tests to support HPUs.

CHANGES

Replaced MultiProcessTestCase with the DistributedTestBase class.
Extended test functionality to include support for HPUs.
Utilized instantiate_device_type_tests with targeted attributes to generate device-specific test instances.
Applied the skipIfHPU decorator to skip tests that are not yet compatible with HPU devices.

cc @H-Huang @awgu @kwen2501 @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @c-p-i-o @ankurneog

pytorch-bot · 2025-01-17T07:23:08Z

🔗 Helpful Links

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

📄 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.

✅ You can merge normally! (1 Unrelated Failure)

As of commit 6a29ea1 with merge base 9c78fb9 ():

FLAKY - The following job failed but was likely due to flakiness present on trunk:

pull / linux-focal-cuda11.8-py3.10-gcc9 / test (distributed, 1, 3, lf.linux.g4dn.12xlarge.nvidia.gpu) (gh) (detected as infra flaky with no log or failing log classifier)

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

Skylion007 · 2025-01-17T15:37:49Z

test/distributed/test_c10d_object_collectives.py

Can we also please properly type annotation this function with callable[P,R] etc? Decorators are important to type properly as they lose all typing info of things they decorate otherwise.

Suggested change

def with_comms(func=None):

def with_comms(func=None):

Skylion007 · 2025-01-17T15:39:15Z

test/distributed/test_c10d_object_collectives.py

Suggested change

devices=["cpu","cuda","hpu"]

devices=("cpu","cuda","hpu")

wconstab · 2025-01-17T15:50:08Z

test/distributed/test_c10d_object_collectives.py

I'm curious if the way DistributedTestBase passes device arg to test classes plays nicely with using @parametrize on test instances- have you tried this?

I have tried using both @parameterize and DistributedTestCase in the same file on different test cases (test/distributed/test_functional_api.py) but I haven't tried using them both on the same testcase so I am unsure about how they interact.

Thanks for pointing this out! I will take a look at it and see if any changes are required :)

ankurneog · 2025-01-21T10:21:30Z

@kwen2501 : Kindly help with the review. thanks

kwen2501 · 2025-01-23T06:40:41Z

Sorry just back to work this week. Will have a look soon.
@ankurneog You can add me or @fegin to the reviewer list next time for Distributed tests. (That helps me search for PRs that need a review.) Thanks!

kwen2501

LGTM. We are very close to land, once the comments are addressed.

test/distributed/test_c10d_object_collectives.py

amathewc · 2025-01-27T03:30:05Z

@kwen2501 : I had addressed your review comments ? Could you take a look?

amathewc · 2025-01-28T04:44:16Z

@ankurneog , @kwen2501 , @fegin : Could you help with the approval and merging ?

amathewc · 2025-02-05T10:24:31Z

@ankurneog , @kwen2501 : Could you help with the merging ?

amathewc · 2025-02-06T04:53:54Z

@pytorchbot merge -i

pytorch-bot · 2025-02-06T04:53:58Z

-i flag is only allowed for users with write permissions

amathewc · 2025-02-06T07:03:31Z

@guangyey : Could you help with retriggering the CI and merge ?

ankurneog · 2025-02-06T10:05:31Z

Sorry just back to work this week. Will have a look soon. @ankurneog You can add me or @fegin to the reviewer list next time for Distributed tests. (That helps me search for PRs that need a review.) Thanks!

@kwen2501 I don't seem to have the permission to directly assign reviewers. Do i need to do anything from my side to enable that ?

guangyey · 2025-02-08T07:26:12Z

@pytorchbot merge

guangyey · 2025-02-08T07:26:33Z

@pytorchbot rebase

pytorchmergebot · 2025-02-08T07:27:51Z

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 · 2025-02-08T07:27:57Z

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

Fix typo to resolve CI issues.

Fix typo to resolve CI issues. Fix issues as per review comments.

pytorchmergebot · 2025-02-08T07:28:00Z

Successfully rebased distributed_changes onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout distributed_changes && git pull --rebase)

pytorchmergebot · 2025-02-08T07:33:09Z

Merge failed

Reason: New commits were pushed while merging. Please rerun the merge command.

Details for Dev Infra team

Raised by workflow job

guangyey · 2025-02-10T01:34:53Z

@pytorchbot merge

pytorchmergebot · 2025-02-10T01:36:32Z

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 · 2025-02-10T03:01:48Z

Merge failed

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

pull / linux-focal-cuda11.8-py3.10-gcc9 / test (distributed, 1, 3, lf.linux.g4dn.12xlarge.nvidia.gpu)

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

amathewc · 2025-02-13T03:41:33Z

@kwen2501 , @guangyey , @ankurneog : Could you help with the merging ? The failure seems to be unrelated.

AnantGulati · 2025-02-13T03:50:37Z

"Try to land this since the failure is unrelated."
@pytorchbot merge

pytorchmergebot · 2025-02-13T03:52:15Z

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

…pytorch#145056) # MOTIVATION To generalize distributed test cases for non-CUDA devices, we are leveraging the DistributedTestBase class introduced in [PR pytorch#138216](pytorch#138216). This new class is derived from MultiProcessTestCase and abstracts the creation/deletion of process groups and other functionality for specific devices. In this PR, we extend the scope of these tests to support HPUs. # CHANGES Replaced MultiProcessTestCase with the DistributedTestBase class. Extended test functionality to include support for HPUs. Utilized instantiate_device_type_tests with targeted attributes to generate device-specific test instances. Applied the skipIfHPU decorator to skip tests that are not yet compatible with HPU devices. Pull Request resolved: pytorch#145056 Approved by: https://github.com/kwen2501, https://github.com/guangyey

pytorch-bot bot added oncall: distributed Add this issue/PR to distributed oncall triage queue topic: not user facing topic category labels Jan 17, 2025

pytorchbot added the open source label Jan 17, 2025

Skylion007 reviewed Jan 17, 2025

View reviewed changes

wconstab reviewed Jan 17, 2025

View reviewed changes

mikaylagawarecki added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jan 22, 2025

kwen2501 self-requested a review January 23, 2025 17:19

kwen2501 approved these changes Jan 23, 2025

View reviewed changes

test/distributed/test_c10d_object_collectives.py Outdated Show resolved Hide resolved

test/distributed/test_c10d_object_collectives.py Outdated Show resolved Hide resolved

test/distributed/test_c10d_object_collectives.py Outdated Show resolved Hide resolved

guangyey approved these changes Feb 8, 2025

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Feb 8, 2025

pytorchmergebot added the merging label Feb 8, 2025

amathewc added 4 commits February 8, 2025 07:27

Update test_c10d_object_collectives.py with DistributedTestBase class

433af1f

Update as per review comments

d52aef8

Update as per review comments

c8ae41e

Fix typo to resolve CI issues.

Update as per review comments

6a29ea1

Fix typo to resolve CI issues. Fix issues as per review comments.

pytorchmergebot force-pushed the distributed_changes branch from 287a9e6 to 6a29ea1 Compare February 8, 2025 07:28

pytorchmergebot removed the merging label Feb 8, 2025

pytorchmergebot added the merging label Feb 10, 2025

pytorchmergebot removed the merging label Feb 10, 2025

pytorchmergebot added the merging label Feb 13, 2025

pytorchmergebot added the Merged label Feb 13, 2025

pytorchmergebot closed this in 936df45 Feb 13, 2025

pytorchmergebot removed the merging label Feb 13, 2025

amathewc deleted the distributed_changes branch February 18, 2025 09:30

ankurneog mentioned this pull request Mar 5, 2025

[RFC] Generalize pytorch content for non-native device execution pytorch/rfcs#66

Open

Update test_c10d_object_collectives.py with DistributedTestBase class #145056

Update test_c10d_object_collectives.py with DistributedTestBase class #145056

Uh oh!

Conversation

amathewc commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MOTIVATION

CHANGES

Uh oh!

pytorch-bot bot commented Jan 17, 2025 • 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/145056

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

Skylion007 Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Skylion007 Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

wconstab Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

AnantGulati Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

ankurneog commented Jan 21, 2025

Uh oh!

kwen2501 commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kwen2501 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amathewc commented Jan 27, 2025

Uh oh!

amathewc commented Jan 28, 2025

Uh oh!

amathewc commented Feb 5, 2025

Uh oh!

amathewc commented Feb 6, 2025

Uh oh!

pytorch-bot bot commented Feb 6, 2025

Uh oh!

amathewc commented Feb 6, 2025

Uh oh!

ankurneog commented Feb 6, 2025

Uh oh!

guangyey commented Feb 8, 2025

Uh oh!

guangyey commented Feb 8, 2025

Uh oh!

pytorchmergebot commented Feb 8, 2025

Merge started

Uh oh!

pytorchmergebot commented Feb 8, 2025

Uh oh!

pytorchmergebot commented Feb 8, 2025

Uh oh!

pytorchmergebot commented Feb 8, 2025

Merge failed

Uh oh!

guangyey commented Feb 10, 2025

Uh oh!

pytorchmergebot commented Feb 10, 2025

Merge started

Uh oh!

pytorchmergebot commented Feb 10, 2025

Merge failed

Uh oh!

amathewc commented Feb 13, 2025

Uh oh!

AnantGulati commented Feb 13, 2025

Uh oh!

pytorchmergebot commented Feb 13, 2025

amathewc commented Jan 17, 2025 •

edited

Loading

pytorch-bot bot commented Jan 17, 2025 •

edited

Loading

Skylion007 Jan 17, 2025 •

edited

Loading

kwen2501 commented Jan 23, 2025 •

edited

Loading