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

Linear with DID loop split #3650

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Linear with DID loop split #3650

wants to merge 4 commits into from

Conversation

Priya2698
Copy link
Collaborator

@Priya2698 Priya2698 commented Dec 28, 2024

This PR adds a test case demonstrating DID loop split for linear.
The test does not require any changes to the LinearOp::evaluate method and works out of the box. DID split on logical domain continues to work for linear using the additional WARs present that squeeze/unsqueeze the DID dimension. Those WARs can be removed once we completely switch to representing device parallelism using allocation and loop domain.

@Priya2698
Copy link
Collaborator Author

!build

@Priya2698 Priya2698 requested review from wujingyue, samnordmann and cowanmeg and removed request for cowanmeg and samnordmann December 28, 2024 06:10
Comment on lines +127 to +128
self.weight = self.define_tensor([d * e, e], contiguity=[True, True])
self.bias = self.define_tensor([d * e], contiguity=[True])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
self.weight = self.define_tensor([d * e, e], contiguity=[True, True])
self.bias = self.define_tensor([d * e], contiguity=[True])
self.weight = self.define_tensor([d * e, e], contiguity=True)
self.bias = self.define_tensor([d * e], contiguity=True)

Also, a question for you: is contiguity=True necessary here? Is there a problem with non-contiguous inputs?

Comment on lines +169 to +171
expected_out_tensor = unsharded_out_tensor.view([b, s, d, e]).permute(2, 0, 1, 3)[
rank : rank + 1
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use shard_tensor here?

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.

2 participants