-
Notifications
You must be signed in to change notification settings - Fork 25.7k
composability test cleanup #145011
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
composability test cleanup #145011
Conversation
minor changes to test public PP api instead of internal/private one and also save a few lines of code for microbatch splitting in the process [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/145011
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 5 PendingAs of commit ea1fc2d with merge base adbbcd8 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
minor changes to test public PP api instead of internal/private one and also save a few lines of code for microbatch splitting in the process cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 d4l3k c-p-i-o [ghstack-poisoned]
minor changes to test public PP api instead of internal/private one and also save a few lines of code for microbatch splitting in the process [ghstack-poisoned]
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.
Looks good
|
|
||
| # Run the pipeline | ||
| if pp_group.rank() == 0: | ||
| pipeline_schedule._step_microbatches(arg_mbs=input_mb) |
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.
Nice thanks for removing! I checked the test folder, it doesn't look like any of our tests are using _step_microbatches anymore
|
@pytorchbot 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 |
|
The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command |
Allows test classes using MPCT to set their own timeout as a class property, which is good enough since the processgroup is shared across test instances and the timeout is set at processgroup init. Also sets a default timeout of 2 minutes, which is probably (?) long enough for reasonable tests, but can be changed if it causes flakyness. It's preferable to have as short default timeout as possible, since when debugging tests getting a timeout quickly helps. Pull Request resolved: #145099 Approved by: https://github.com/d4l3k, https://github.com/fduwjj ghstack dependencies: #145010, #145011
minor changes to test public PP api instead of internal/private one and also save a few lines of code for microbatch splitting in the process ghstack-source-id: b173162 Pull Request resolved: pytorch/pytorch#145011
Stack from ghstack (oldest at bottom):
minor changes to test public PP api instead of internal/private one and
also save a few lines of code for microbatch splitting in the process
cc @H-Huang @awgu @kwen2501 @wanchaol @fegin @fduwjj @wz337 @d4l3k @c-p-i-o