Skip to content

Conversation

@fduwjj
Copy link
Contributor

@fduwjj fduwjj commented Sep 12, 2024

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
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@fduwjj fduwjj closed this Sep 12, 2024
facebook-github-bot pushed a commit that referenced this pull request Sep 12, 2024
…the correct code structure (#132931) (#2384)

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
bigfootjon pushed a commit that referenced this pull request 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 pushed a commit that referenced this pull request 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

fbshipit-source-id: c31184311a0f72044f506e75a78d5dac0471bc41
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