Replacing explicit backend search with api call by AnantGulati · Pull Request #144944 · pytorch/pytorch

AnantGulati · 2025-01-16T06:23:45Z

Following up on PR #138216

In this PR we are modifying the internal structure of common distributed class DistributedTestBase to use an inbuilt function get_default_backend_for_device (#140536).

We have also refactored test file test_functional_api to remove all explicit calls to test_hpu. to add support for a new device to this file a user must only add the device to list of devices. This supports both in-tree and out of tree devices

This PR also adds a setter for world size inside multiprocess test case. This allows for test cases to redefine world size for different test cases in the same class and removes the need to add a new class simply to change properties like world size as shown in test_functional_api

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

pytorch-bot · 2025-01-16T06:23:48Z

🔗 Helpful Links

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

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

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

pytorch-bot · 2025-01-16T06:27:00Z

❌ 🤖 pytorchbot command failed:

Got EOF while in a quoted string```
Try `@pytorchbot --help` for more info.

AnantGulati · 2025-01-16T06:27:18Z

@pytorchbot label "topic: not user facing"

test/distributed/test_functional_api.py

AnantGulati · 2025-01-17T03:50:51Z

@kwen2501 could you please review and help land this PR.
Thanks

kwen2501

LGTM. We are very close to land once we clean up a couple points in the comments

kwen2501 · 2025-01-17T08:02:25Z

torch/testing/_internal/common_distributed.py

new line

Also, how would this setter interact with the world_size function above?

It should interact with the above function such that it allows us to change world size for only one test case in the test file as shown here: https://github.com/pytorch/pytorch/pull/144944/files/25e3ab6c2c314457ef62b8e610f572e45e7724c7#r1919782578

kwen2501 · 2025-01-17T08:04:06Z

test/distributed/test_functional_api.py

What is this new test for?

It's not a new test, I have simply shifted it to the class TestCollectivesWithDistributedBackend, from it's original class as the only difference between the original class and TestCollectivesWithDistributedBackend is the changed world size.

This class was causing error while refactoring the code so I removed it and added the world size setter which will allow us to modify world size in common distributed.

test/distributed/test_functional_api.py

AnantGulati · 2025-01-17T09:06:58Z

test/distributed/test_functional_api.py

We have added this line to change the world size only in this test case, this removes the need of creating a new class just to change one property without impacting the other classes

Do you mind reverting the setting of world_size here and restore the original class, OR
use world_size = 4 instead of self.world_size ?

Today the change may be fine, but in case class-based Test fixture is used, any setting to self in one test method would impact the rest tests, because class-based Test fixture will not recreate a new test object per test.

AnantGulati · 2025-01-20T04:39:50Z

@pytorchbot merge

pytorch-bot · 2025-01-20T04:39:54Z

Pull workflow has not been scheduled for the PR yet. It could be because author doesn't have permissions to run those or skip-checks keywords were added to PR/commits, aborting merge. Please get/give approval for the workflows and/or remove skip ci decorators before next merge attempt. If you think this is a mistake, please contact PyTorch Dev Infra.

AnantGulati · 2025-01-20T10:08:07Z

@pytorchbot rebase

pytorchmergebot · 2025-01-20T10:10:24Z

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

pytorchmergebot · 2025-01-20T10:10:29Z

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

kwen2501

I am about to land this PR, but it seems one small change needs revert. Can you please help? Thank you!

kwen2501 · 2025-03-10T19:10:25Z

test/distributed/test_functional_api.py

Do you mind reverting the setting of world_size here and restore the original class, OR
use world_size = 4 instead of self.world_size ?

Today the change may be fine, but in case class-based Test fixture is used, any setting to self in one test method would impact the rest tests, because class-based Test fixture will not recreate a new test object per test.

AnantGulati · 2025-03-11T05:20:56Z

Do you mind reverting the setting of world_size here and restore the original class, OR
use world_size = 4 instead of self.world_size ?

Today the change may be fine, but in case class-based Test fixture is used, any setting to self in one test method would impact the rest tests, because class-based Test fixture will not recreate a new test object per test.

I have maintained the initial class structure to avoid this problem.

I have also changed the structure so that TestDistributedBackendCollectivesWithWorldSize4 inherits directly from DistributedTestBase.

This is cleaner as the set up steps which were initially present in TestDistributedBackendCollective have been abstracted in DistributedTestBase. This also solves the other errors which we were hitting due to inheriting classes which were intialized in instantiate_device_type_tests.

@kwen2501 I'd love to hear if you think any further adjustments are required. Thanks

AnantGulati · 2025-03-14T06:56:40Z

@kwen2501 Could you please help with this PR

AnantGulati · 2025-03-17T09:09:33Z

@kwen2501 all suggested changes have been added. Can we proceed with this PR?

AnantGulati · 2025-03-20T05:25:58Z

@kwen2501 Could you please take a look at this PR. It has been pending for some time now
Thanks

guangyey · 2025-03-21T06:29:45Z

@pytorchbot rebase

pytorchmergebot · 2025-03-21T06:31:14Z

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

pytorchmergebot · 2025-03-21T06:31:19Z

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

AnantGulati · 2025-04-02T09:23:17Z

@kwen2501 Could you please help with this review. All changes you suggested have been implemented

pytorchmergebot · 2025-05-01T05:31:44Z

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

cyyever · 2025-05-01T06:16:40Z

@pytorchmergebot merge -i

pytorchmergebot · 2025-05-01T06:18:47Z

Merge started

Your change will be merged while ignoring the following 0 checks:

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-05-01T07:11: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, ephemeral.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

guangyey · 2025-05-05T08:48:53Z

@pytorchbot merge

pytorchmergebot · 2025-05-05T08:51:25Z

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-05-05T08:56:53Z

Merge failed

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

Lint / Link checks / Lint URLs / linux-job

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

guangyey · 2025-05-09T05:27:26Z

@AnantGulati please resolve the conflicts.

guangyey · 2025-05-09T05:31:28Z

@pytorchbot rebase

pytorchmergebot · 2025-05-09T05:33:03Z

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

pytorchmergebot · 2025-05-09T05:33:05Z

Rebase failed due to Command git -C /home/runner/work/pytorch/pytorch rebase refs/remotes/origin/viable/strict pull/144944/head returned non-zero exit code 1

Rebasing (1/8)
Auto-merging torch/testing/_internal/common_distributed.py
CONFLICT (content): Merge conflict in torch/testing/_internal/common_distributed.py
error: could not apply 59000a74fca... reverting class structure
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Could not apply 59000a74fca... reverting class structure

Raised by https://github.com/pytorch/pytorch/actions/runs/14922140404

AnantGulati · 2025-06-10T04:25:20Z

@pytorchbot rebase

pytorchmergebot · 2025-06-10T04:27:10Z

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

pytorchmergebot · 2025-06-10T04:27:13Z

Rebase failed due to Command git -C /home/runner/work/pytorch/pytorch rebase refs/remotes/origin/viable/strict pull/144944/head returned non-zero exit code 1

Rebasing (1/3)
Auto-merging test/distributed/test_functional_api.py
Auto-merging torch/testing/_internal/common_distributed.py
CONFLICT (content): Merge conflict in torch/testing/_internal/common_distributed.py
error: could not apply 59000a74fca... reverting class structure
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Could not apply 59000a74fca... reverting class structure

Raised by https://github.com/pytorch/pytorch/actions/runs/15550541252

github-actions · 2025-08-09T04:41:39Z

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

pytorch-bot bot added the oncall: distributed Add this issue/PR to distributed oncall triage queue label Jan 16, 2025

pytorch-bot bot added the topic: not user facing topic category label Jan 16, 2025

pytorchbot added the open source label Jan 16, 2025

ankurneog reviewed Jan 16, 2025

View reviewed changes

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

kwen2501 approved these changes Jan 17, 2025

View reviewed changes

AnantGulati commented Jan 17, 2025

View reviewed changes

pytorchmergebot force-pushed the AnantGulati_distributed_test_case_changes branch from 4feeb3f to a4e58d2 Compare January 20, 2025 10:10

guangyey approved these changes Jan 23, 2025

View reviewed changes

ankurneog mentioned this pull request Mar 5, 2025

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

Open

kwen2501 requested changes Mar 10, 2025

View reviewed changes

soulitzer added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Mar 10, 2025

AnantGulati force-pushed the AnantGulati_distributed_test_case_changes branch from 64d64cc to 2003796 Compare March 11, 2025 04:58

pytorchmergebot force-pushed the AnantGulati_distributed_test_case_changes branch from 2003796 to ee5e207 Compare March 21, 2025 06:31

AnantGulati added 2 commits May 1, 2025 05:31

reverting class structure

59000a7

fixing lint error

89072b2

pytorchmergebot force-pushed the AnantGulati_distributed_test_case_changes branch from c08b452 to 89072b2 Compare May 1, 2025 05:31

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

pytorchmergebot added the merging label May 1, 2025

pytorchmergebot removed the merging label May 1, 2025

adding CI fix

1e64294

pytorch-bot bot removed the ciflow/trunk Trigger trunk jobs on your pull request label May 5, 2025

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

pytorchmergebot added the merging label May 5, 2025

pytorchmergebot removed the merging label May 5, 2025

Merge branch 'main' into AnantGulati_distributed_test_case_changes

4faa9b2

pytorch-bot bot removed the ciflow/trunk Trigger trunk jobs on your pull request label May 9, 2025

github-actions bot added the Stale label Aug 9, 2025

github-actions bot closed this Sep 8, 2025

Conversation

AnantGulati commented Jan 16, 2025 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Jan 16, 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/144944

Uh oh!

pytorch-bot bot commented Jan 16, 2025

Uh oh!

AnantGulati commented Jan 16, 2025

Uh oh!

Uh oh!

AnantGulati commented Jan 17, 2025

Uh oh!

kwen2501 left a comment

Choose a reason for hiding this comment

Uh oh!

kwen2501 Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

AnantGulati Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

kwen2501 Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

AnantGulati Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AnantGulati Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

kwen2501 Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

AnantGulati commented Jan 20, 2025

Uh oh!

pytorch-bot bot commented Jan 20, 2025

Uh oh!

AnantGulati commented Jan 20, 2025

Uh oh!

pytorchmergebot commented Jan 20, 2025

Uh oh!

pytorchmergebot commented Jan 20, 2025

Uh oh!

kwen2501 left a comment

Choose a reason for hiding this comment

Uh oh!

kwen2501 Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

AnantGulati commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AnantGulati commented Mar 14, 2025

Uh oh!

AnantGulati commented Mar 17, 2025

Uh oh!

AnantGulati commented Mar 20, 2025

Uh oh!

guangyey commented Mar 21, 2025

Uh oh!

pytorchmergebot commented Mar 21, 2025

Uh oh!

pytorchmergebot commented Mar 21, 2025

Uh oh!

AnantGulati commented Apr 2, 2025

Uh oh!

pytorchmergebot commented May 1, 2025

Uh oh!

cyyever commented May 1, 2025

Uh oh!

pytorchmergebot commented May 1, 2025

Merge started

Uh oh!

pytorchmergebot commented May 1, 2025

AnantGulati commented Jan 16, 2025 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Jan 16, 2025 •

edited

Loading

AnantGulati commented Mar 11, 2025 •

edited

Loading