[distributed][sharded_tensor] Move local_shards check from ShardedTensorBase to ShardedTensor #100197

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

Closed

IvanKobzarev wants to merge 6 commits into gh/ivankobzarev/126/base from gh/ivankobzarev/126/head

Contributor

IvanKobzarev commented Apr 27, 2023 •

edited

Loading

Stack from ghstack (oldest at bottom):

-> [distributed][sharded_tensor] Move local_shards check from ShardedTensorBase to ShardedTensor #100197

Differential Revision: D45369211


          [distributed][sharded_tensor] Move local_shards check from ShardedTen…

19c59a1

…sorBase to ShardedTensor

[ghstack-poisoned]

IvanKobzarev requested review from H-Huang, awgu, fegin, kwen2501, mrshenli, rohan-varma, wanchaol and zhaojuanmao as code owners

April 27, 2023 20:13

pytorch-bot bot commented Apr 27, 2023 •

edited

Loading

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

❗ 2 Active SEVs

There are 2 currently active SEVs. If your PR is affected, please view them below:

❌ 2 New Failures

As of commit 961619c:

NEW FAILURES - The following jobs have failed:

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

IvanKobzarev requested review from d4l3k and kiukchung as code owners

April 27, 2023 20:13

pytorch-bot bot added the release notes: distributed (sharded) label

IvanKobzarev added a commit that referenced this pull request


          [distributed][sharded_tensor] Move local_shards check from ShardedTen…

32f82c0

…sorBase to ShardedTensor

ghstack-source-id: 0b2d834
Pull Request resolved: #100197

Contributor Author

IvanKobzarev commented Apr 27, 2023

@IvanKobzarev has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.


          Update on "[distributed][sharded_tensor] Move local_shards check from…

ba6cfb2

… ShardedTensorBase to ShardedTensor"

Differential Revision: [D45369211](https://our.internmc.facebook.com/intern/diff/D45369211)

[ghstack-poisoned]

IvanKobzarev added a commit that referenced this pull request


          [distributed][sharded_tensor] Move local_shards check from ShardedTen…

daa480a

…sorBase to ShardedTensor

ghstack-source-id: 8e39c06
Pull Request resolved: #100197

Contributor Author

IvanKobzarev commented Apr 28, 2023

@IvanKobzarev has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

fduwjj added ciflow/trunk ciflow/periodic labels

fduwjj approved these changes

View reviewed changes

Contributor

fduwjj left a comment

Unblock. Overall, this idea of this change looks good but it looks like that we are copying same logic to ShardedTensor, it would be great if we can refactor it a little bit so that we don't have duplicate code and consolidate common logics.

Please also make sure internal tests also all pass. Thanks!


          Update on "[distributed][sharded_tensor] Move local_shards check from…

b9b3cbc

… ShardedTensorBase to ShardedTensor"

Differential Revision: [D45369211](https://our.internmc.facebook.com/intern/diff/D45369211)

[ghstack-poisoned]

Contributor Author

IvanKobzarev commented May 1, 2023

@IvanKobzarev has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.


          Update on "[distributed][sharded_tensor] Move local_shards check from…

132a13b

… ShardedTensorBase to ShardedTensor"

Differential Revision: [D45369211](https://our.internmc.facebook.com/intern/diff/D45369211)

[ghstack-poisoned]

Contributor Author

IvanKobzarev commented May 1, 2023

@IvanKobzarev has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.


          Update on "[distributed][sharded_tensor] Move local_shards check from…

4dd1ab8

… ShardedTensorBase to ShardedTensor"

Differential Revision: [D45369211](https://our.internmc.facebook.com/intern/diff/D45369211)

[ghstack-poisoned]

Contributor Author

IvanKobzarev commented May 1, 2023

@IvanKobzarev has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.


          Update on "[distributed][sharded_tensor] Move local_shards check from…

961619c

… ShardedTensorBase to ShardedTensor"

Differential Revision: [D45369211](https://our.internmc.facebook.com/intern/diff/D45369211)

[ghstack-poisoned]

IvanKobzarev added a commit that referenced this pull request


          [distributed][sharded_tensor] Move local_shards check from ShardedTen…

af7dee0

…sorBase to ShardedTensor

ghstack-source-id: 3a08edf
Pull Request resolved: #100197

Contributor Author

IvanKobzarev commented May 2, 2023

@IvanKobzarev has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Contributor Author

IvanKobzarev commented May 2, 2023

@pytorchbot merge

pytorchmergebot added the merging label

Collaborator

pytorchmergebot commented May 2, 2023

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

Collaborator

pytorchmergebot commented May 2, 2023

Merge failed

Reason: 2 jobs have failed, first few of them are: periodic / cuda12.1-py3.10-gcc7-sm86-periodic-dynamo-benchmarks / test (dynamo_eager_torchbench, 1, 1, linux.g5.4xlarge.nvidia.gpu), periodic / cuda12.1-py3.10-gcc7-sm86-periodic-dynamo-benchmarks / test (dynamic_aot_eager_huggingface, 1, 1, linux.g5.4xlarge.nvidia.gpu)

Details for Dev Infra team

Raised by workflow job

pytorchmergebot removed the merging label

Contributor Author

IvanKobzarev commented May 2, 2023

@pytorchbot merge -f "unrelated CI failures"

pytorchmergebot added the merging label

Collaborator

pytorchmergebot commented May 2, 2023

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

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 added Merged and removed merging labels

pytorchmergebot closed this in

4582ceb

valentinandrei pushed a commit to valentinandrei/pytorch that referenced this pull request


          [distributed][sharded_tensor] Move local_shards check from ShardedTen…

…sorBase to ShardedTensor (pytorch#100197)

Differential Revision: [D45369211](https://our.internmc.facebook.com/intern/diff/D45369211)
Pull Request resolved: pytorch#100197
Approved by: https://github.com/fduwjj

facebook-github-bot deleted the gh/ivankobzarev/126/head branch

June 8, 2023 17:20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

fduwjj fduwjj approved these changes

mrshenli Awaiting requested review from mrshenli

zhaojuanmao Awaiting requested review from zhaojuanmao

rohan-varma Awaiting requested review from rohan-varma

H-Huang Awaiting requested review from H-Huang

awgu Awaiting requested review from awgu

kwen2501 Awaiting requested review from kwen2501

wanchaol Awaiting requested review from wanchaol

fegin Awaiting requested review from fegin

kiukchung Awaiting requested review from kiukchung

d4l3k Awaiting requested review from d4l3k

Labels

ciflow/periodic ciflow/trunk Merged release notes: distributed (sharded)