-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[Easy][DTensor] Rename args_sharding to args_schema for OpSchema __str__ #132187
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/132187
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 9bf173b with merge base 3864a2d ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
||
def __str__(self) -> str: | ||
args_sharding: List[str] = [] | ||
args_schema: List[str] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why we don't use args_schema
is that it is confliciting with OpSchema.args_schema
, so we want to distinguish that, why? args_schema
could consists of non Tensor/DTensor arguments, while args_sharding means we only want to include DTensor sharding into the list, excluding non-Tensor/DTensor args
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow up question: I thought everything else also get append to args_sharding from here:
https://github.com/pytorch/pytorch/blob/main/torch/distributed/_tensor/_op_schema.py#L289-L290
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm actually you are right, my bad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sgtm!
@pytorchmergebot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Looks like we don't use the name
args_sharding
anywhere else so just changing it toargs_schema
for naming consistencycc @XilunWu @H-Huang @awgu @kwen2501 @wanchaol @fegin @fduwjj @wconstab @d4l3k @c-p-i-o @tianyu-l @chauhang