Skip to content

reduce redundant graph in collective op #9044

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

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

Conversation

zpcore
Copy link
Collaborator

@zpcore zpcore commented Apr 25, 2025

In all-reduce lowering, we create the redundant graph as below (marked with REMOVE):

image

If we don't plan to scale on the final reduced output, the subgraph (marked with REMOVE) is unnecessary. This PR is to remove the redundant graph when scale=1.0.

@zpcore zpcore marked this pull request as ready for review April 25, 2025 22:24
@zpcore zpcore added the distributed SPMD and other distributed things. label Apr 25, 2025
@zpcore zpcore requested review from rpsilva-aws and tengyifei April 25, 2025 22:45
Copy link
Collaborator

@tengyifei tengyifei left a comment

Choose a reason for hiding this comment

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

LGTM. But note that a lot of collectives-related tests all failed.

@zpcore zpcore force-pushed the piz/clean_reduce branch from 9a1b603 to a20ffa1 Compare April 26, 2025 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed SPMD and other distributed things.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants