Skip to content

[DCP]Add unit test for tp checkpoint #108286

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
wants to merge 1 commit into from
Closed

[DCP]Add unit test for tp checkpoint #108286

wants to merge 1 commit into from

Conversation

wz337
Copy link
Contributor

@wz337 wz337 commented Aug 30, 2023

As title.

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Aug 30, 2023
@pytorch-bot
Copy link

pytorch-bot bot commented Aug 30, 2023 •
edited
Loading

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit e213847 with merge base 7349e8c (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

@wz337 wz337 marked this pull request as ready for review August 30, 2023 21:27
@wz337 wz337 force-pushed the tp_checkpoint_example branch from a125f3c to d4aa12c Compare August 30, 2023 22:05
wanchaol
wanchaol approved these changes Aug 30, 2023
View reviewed changes
Copy link
Collaborator

@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.

LGTM. As a follow up, can you also add some examples in this file? https://github.com/pytorch/pytorch/blob/main/torch/distributed/_tensor/examples/checkpoint_example.py#L136

test/distributed/checkpoint/test_tp_checkpoint.py Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: we can possibly reuse the same model defined in https://github.com/pytorch/pytorch/blob/main/torch/testing/_internal/distributed/_tensor/common_dtensor.py#L53

fduwjj
fduwjj reviewed Aug 31, 2023
View reviewed changes
test/distributed/checkpoint/test_tp_checkpoint.py Outdated
Copy link
Contributor

@fduwjj fduwjj Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can we add a test case where we use Meta device? Basically we materialize it after TP sharding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add that in follow up PR.

@wz337 wz337 force-pushed the tp_checkpoint_example branch from d4aa12c to f6c1926 Compare August 31, 2023 20:00
@wz337
Copy link
Contributor Author

wz337 commented Aug 31, 2023

@pytorchmergebot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 31, 2023
@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: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@wz337 wz337 force-pushed the tp_checkpoint_example branch from f6c1926 to e213847 Compare August 31, 2023 21:11
@wz337
Copy link
Contributor Author

wz337 commented Aug 31, 2023

@pytorchmergebot 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: 1 jobs have failed, first few of them are: trunk / linux-focal-rocm5.6-py3.8 / test (default, 2, 3, linux.rocm.gpu)

Details for Dev Infra team Raised by workflow job

@wz337
Copy link
Contributor Author

wz337 commented Sep 1, 2023

@pytorchmergebot merge -f "unrelated trunk failure"

@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

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

Reviewers

@fduwjj fduwjj fduwjj approved these changes

@wanchaol wanchaol wanchaol approved these changes

@mrshenli mrshenli Awaiting requested review from mrshenli

@zhaojuanmao zhaojuanmao Awaiting requested review from zhaojuanmao

@rohan-varma rohan-varma Awaiting requested review from rohan-varma

@H-Huang H-Huang Awaiting requested review from H-Huang

@awgu awgu Awaiting requested review from awgu

@kwen2501 kwen2501 Awaiting requested review from kwen2501

@fegin fegin Awaiting requested review from fegin

Assignees

No one assigned

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged topic: not user facing topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@wz337 @pytorchmergebot @fduwjj @wanchaol