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.

Sign up for GitHub

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

Jump to bottom

[dtensor] remove output_ prefix from OpStrategy properties #126359

Closed
wants to merge 5 commits into from
Closed

[dtensor] remove output_ prefix from OpStrategy properties #126359

wants to merge 5 commits into from

Conversation

tianyu-l
Copy link
Contributor

@tianyu-l tianyu-l commented May 16, 2024 •
edited

@tianyu-l tianyu-l mentioned this pull request May 16, 2024
Closed
@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented May 16, 2024 •
edited

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/126359

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures, 1 Unrelated Failure

As of commit 70a00b7 with merge base 6550386 (image):

NEW FAILURES - The following jobs have failed:

UNSTABLE - The following job failed but was likely due to flakiness present on trunk and has been marked as unstable:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added ciflow/inductor oncall: distributed Add this issue/PR to distributed oncall triage queue labels May 16, 2024
tianyu-l added a commit that referenced this pull request May 16, 2024
@tianyu-l
[dtensor] rename OpStrategy property output_ndim to ndim, output_shap…
b694e85 
…e to shape

ghstack-source-id: a8956263c6d9da571294646c9e55a99fae29af34
Pull Request resolved: #126359
@tianyu-l tianyu-l added ciflow/trunk Trigger trunk jobs on your pull request release notes: distributed (dtensor) release notes category labels May 16, 2024
wanchaol
wanchaol approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@wanchaol wanchaol left a comment

Choose a reason for hiding this comment

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

sg!

wz337
wz337 reviewed May 16, 2024
View reviewed changes
torch/distributed/_tensor/op_schema.py
@@ -153,21 +153,13 @@ def output_mesh_shape(self):
assert output_spec[0] is not None
return output_spec[0].mesh.shape

@property
def output_ndim(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed in the PR but since we remove the output prefix, should we rename output_mesh_shape to mesh_shape for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good point, I missed that. Let me change that in this PR, as it only appears locally in this file.

@tianyu-l
Update on "[dtensor] rename OpStrategy property output_ndim to ndim, …
7796686 
…output_shape to shape"

cc mrshenli pritamdamania87 zhaojuanmao satgera gqchen aazzolini osalpekar jiayisuse H-Huang kwen2501 awgu penguinwu fegin XilunWu wanchaol fduwjj wz337 wconstab yf225 chauhang d4l3k

[ghstack-poisoned]
tianyu-l added a commit that referenced this pull request May 16, 2024
@tianyu-l
[dtensor] remove output_ prefix from OpStrategy properties
08ddfb9 
ghstack-source-id: fb24ae37573943f6ff0f767d66eca9d4bb2bce8e
Pull Request resolved: #126359
@tianyu-l tianyu-l changed the title [dtensor] rename OpStrategy property output_ndim to ndim, output_shape to shape [dtensor] remove output_ prefix from OpStrategy properties May 16, 2024
@tianyu-l
Copy link
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your 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

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: Command git -C /home/runner/work/pytorch/pytorch cherry-pick -x 08ddfb92b94a6e05630edf9c24991234e9b0fc65 returned non-zero exit code 1

Auto-merging torch/distributed/_tensor/op_schema.py
CONFLICT (content): Merge conflict in torch/distributed/_tensor/op_schema.py
Auto-merging torch/distributed/_tensor/ops/view_ops.py
CONFLICT (content): Merge conflict in torch/distributed/_tensor/ops/view_ops.py
error: could not apply 08ddfb92b94... [dtensor] remove output_ prefix from OpStrategy properties
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
Details for Dev Infra team Raised by workflow job

@tianyu-l
Update on "[dtensor] remove output_ prefix from OpStrategy properties"
20a0e98 
cc mrshenli pritamdamania87 zhaojuanmao satgera gqchen aazzolini osalpekar jiayisuse H-Huang kwen2501 awgu penguinwu fegin XilunWu wanchaol fduwjj wz337 wconstab yf225 chauhang d4l3k

[ghstack-poisoned]
tianyu-l added a commit that referenced this pull request May 17, 2024
@tianyu-l
[dtensor] remove output_ prefix from OpStrategy properties
c3bf482 
ghstack-source-id: 1c8c660c4fd6949f53c1aca47ba6ddfdfc034665
Pull Request resolved: #126359
@tianyu-l
Copy link
Contributor Author

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@tianyu-l
Update
26e2b1f 
[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Successfully rebased gh/tianyu-l/17/orig onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/126359)

pytorchmergebot pushed a commit that referenced this pull request May 19, 2024
@tianyu-l
[dtensor] remove output_ prefix from OpStrategy properties
acd46c9 
ghstack-source-id: dc1abcb6dd645cac20b6a7715e85b24d210a5b1f
Pull Request resolved: #126359
@tianyu-l
Copy link
Contributor Author

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@tianyu-l
Update
70a00b7 
[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Successfully rebased gh/tianyu-l/17/orig onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/126359)

pytorchmergebot pushed a commit that referenced this pull request May 20, 2024
@tianyu-l
[dtensor] remove output_ prefix from OpStrategy properties
8ad1828 
ghstack-source-id: 19bc5e5c60574d15a3cb49cbdf8d7ca41c349efb
Pull Request resolved: #126359
@tianyu-l
Copy link
Contributor Author

@pytorchbot merge -f "unrelated CI failures"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@wz337 wz337 mentioned this pull request May 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wz337 wz337 wz337 left review comments

@wanchaol wanchaol wanchaol approved these changes

@XilunWu XilunWu XilunWu approved these changes

Assignees
No one assigned
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: distributed (dtensor) release notes category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tianyu-l @pytorchmergebot @wanchaol @XilunWu @wz337