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.

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

Remove Option for ProcessGroup and Expose backend Options to reflect the correct code structure (#132931) (#2384) #2387

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bigfootjon
Copy link
Member

Summary:

X-link: pytorch/pytorch#135653

We introduced the dispatchable backend for a ProcessGroup and collective in pytorch/pytorch#86225. This PR is a follow-up cleanup to clean up the option of a ProcessGroup and ask users to either set timeout or backend later on or directly create backend after creating a PG.

Also PGNCCL is using option class from ProcessGroup but we actually should use Option from backend class. So this PR is to make the type or name to be aligned with what we are doing in cpp side. I don't change the signature for the public API, so they still use args named "pg_options"

We need to make changes to the test to make it aligned with the change.

This is try to reland D62008954 by fixing internal errors.
ghstack-source-id: 242088446

Reviewed By: wz337, H-Huang

Differential Revision: D62483294

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 12, 2024
…the correct code structure (#132931) (#2384)

Summary:
Pull Request resolved: #2384

X-link: pytorch/pytorch#135653

We introduced the dispatchable backend for a ProcessGroup and collective in pytorch/pytorch#86225. This PR is a follow-up cleanup to clean up the option of a ProcessGroup and ask users to either set timeout or backend later on or directly create backend after creating a PG.

Also PGNCCL is using option class from ProcessGroup but we actually should use Option from backend class. So this PR is to make the type or name to be aligned with what we are doing in cpp side. I don't change the signature for the public API, so they still use args named "pg_options"

We need to make changes to the test to make it aligned with the change.

This is try to reland D62008954 by fixing internal errors.
ghstack-source-id: 242088446

Reviewed By: wz337, H-Huang

Differential Revision: D62483294
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D62483294

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants