Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow override of backend in dist.new_group() #18595

Closed
wants to merge 1 commit into from
Closed

Allow override of backend in dist.new_group() #18595

wants to merge 1 commit into from
+133 −10

Conversation

pietern
Copy link
Contributor

@pietern pietern commented Mar 28, 2019

Summary:
There is no need to force the backend to be the same as the global
process group, as long as the backend is "nccl" or "gloo".

Differential Revision: D14657204

@pietern pietern added the oncall: distributed Add this issue/PR to distributed oncall triage queue label Apr 1, 2019
apaszke
apaszke reviewed Apr 4, 2019
View reviewed changes
torch/distributed/distributed_c10d.py Outdated
Checks if the Gloo backend is available.

"""
return True
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems wrong? IIRC you can build PyTorch without gloo, so we should have a check for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@apaszke You can, but right now it won't build c10d, so it won't load torch.distributed at all. This should be fix in and by itself and then this function can be updated as well (added #18851).

@pietern pietern mentioned this pull request Apr 4, 2019
Closed
@pietern
Copy link
Contributor Author

pietern commented Apr 4, 2019

@apaszke Updated is_gloo_available to do the right thing regardless of build configuration.

Added docstring for new backend kwarg to new_group.

@pietern @facebook-github-bot
Allow override of backend in dist.new_group() (pytorch#18595)
0312889 
Summary:
Pull Request resolved: pytorch#18595

There is no need to force the backend to be the same as the global
process group, as long as the backend is "nccl" or "gloo".

Reviewed By: mrshenli

Differential Revision: D14657204

fbshipit-source-id: d8764874e8cfc5306c00e1ca9a951ae88edb7875
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 7a19d3c.

@pietern pietern deleted the export-D14657204 branch April 4, 2019 23:43
@pietern pietern mentioned this pull request Jul 2, 2019
Closed
@ppwwyyxx ppwwyyxx mentioned this pull request Jul 24, 2019
Closed
facebook-github-bot pushed a commit that referenced this pull request Jul 26, 2019
@ppwwyyxx @facebook-github-bot
Update distributed.rst (#23289)
23f963e 
Summary:
Different backend is supported since #18595
Pull Request resolved: #23289

Differential Revision: D16528229

Pulled By: soumith

fbshipit-source-id: 57753e84c015817661ba30835278ee3a899aa2d0
@densechen
Copy link

Is there any demonstration of the usage of new_group()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apaszke apaszke apaszke left review comments

@mrshenli mrshenli Awaiting requested review from mrshenli

Assignees
No one assigned
Labels
oncall: distributed Add this issue/PR to distributed oncall triage queue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pietern @facebook-github-bot @densechen @apaszke