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

Add group as an argument in broadcast ops #2522

Conversation

GindaChen
Copy link
Contributor

Follow up with #2501.

Changes

In communication_ops.py, add group as an argument in the broadcast auxiliary function family. This helps designate the group to communicate (for example, a designated tensor-parallel group, or pipeline-parallel group) in the future.

Copy link
Collaborator

@zhuohan123 zhuohan123 left a comment

Choose a reason for hiding this comment

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

LGTM! Please add the type annotation and I think this PR is good to go!

@@ -86,47 +86,55 @@ def tensor_model_parallel_gather(input_: torch.Tensor,
return output_tensor


def broadcast(input_: torch.Tensor, src: int = 0):
def broadcast(input_: torch.Tensor, src: int = 0, group=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add type annotation, and ditto below

Suggested change
def broadcast(input_: torch.Tensor, src: int = 0, group=None):
def broadcast(input_: torch.Tensor, src: int = 0, group: Optional[ProcessGroup] = None):

Copy link
Contributor Author

@GindaChen GindaChen Jan 20, 2024

Choose a reason for hiding this comment

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

Thanks for reviewing! I committed the new changes as you suggested (with an import of the ProcessGroup symbol).

vllm/model_executor/parallel_utils/communication_op.py Outdated Show resolved Hide resolved
vllm/model_executor/parallel_utils/communication_op.py Outdated Show resolved Hide resolved
@zhuohan123 zhuohan123 merged commit 5b23c3f into vllm-project:main Jan 21, 2024
16 checks passed
@GindaChen GindaChen deleted the refactor-add-group-selector-in-broadcast branch January 21, 2024 00:07
hongxiayang pushed a commit to hongxiayang/vllm that referenced this pull request Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants