change: allow smdistributed to be enabled with torch_distributed. #4129

viclzhu · 2023-09-19T19:09:57Z

Description of changes:
Update estimator to allow smdistributed to be enabled with torch_distributed for a new smdistributed release that supports using smdistributed with torch_distributed.
Disable launching of jobs with smddp on p5 instances or using the p5 image (2.0.1-gpu-py310-cu121-ubuntu20.04-sagemaker-pr-3303) without torch_distributed enabled.

Testing done:
Ran and passed tests/unit/test_estimator.py, tests/unit/test_pytorch.py and tests/unit/test_fw_utils.py locally.

General

I have read the CONTRIBUTING doc
I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
I used the commit message format described in CONTRIBUTING
I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
I have checked that my tests are not configured for a specific region or account (if appropriate)
I have used unique_name_from_base to create resource names in integ tests (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

viclzhu · 2023-09-19T19:30:50Z

cc: @rahul003

rohithn1 · 2023-10-02T22:40:12Z

Note: The following unit tests failed before my and @viclzhu's commits.
======================================================= short test summary info ========================================================
FAILED tests/unit/test_estimator.py::test_local_mode - ValueError: Failed to get the Region information from the default config. Plea...
FAILED tests/unit/test_estimator.py::test_local_mode_file_output_path - ValueError: Failed to get the Region information from the def...
FAILED tests/unit/test_estimator.py::test_insert_invalid_source_code_args - ValueError: Must setup local AWS configuration with a reg...
FAILED tests/unit/test_estimator.py::test_output_path_default_bucket_and_prefix_combinations - ValueError: Failed to get the Region i...
FAILED tests/unit/test_estimator.py::test_stage_user_code_in_s3_default_bucket_and_prefix_combinations[s3://output-bucket/output-prefix/output-prefix2-s3://code-bucket/code-prefix/code-prefix2-expected__without_user_input__with_default_bucket_and_default_prefix0-expected__without_user_input__with_default_bucket_only0-expected__with_user_input__with_default_bucket_and_prefix0-expected__with_user_input__with_default_bucket_only0]
FAILED tests/unit/test_estimator.py::test_stage_user_code_in_s3_default_bucket_and_prefix_combinations[s3://output-bucket/output-prefix/output-prefix2-None-expected__without_user_input__with_default_bucket_and_default_prefix1-expected__without_user_input__with_default_bucket_only1-expected__with_user_input__with_default_bucket_and_prefix1-expected__with_user_input__with_default_bucket_only1]
FAILED tests/unit/test_estimator.py::test_stage_user_code_in_s3_default_bucket_and_prefix_combinations[None-s3://code-bucket/code-prefix/code-prefix2-expected__without_user_input__with_default_bucket_and_default_prefix2-expected__without_user_input__with_default_bucket_only2-expected__with_user_input__with_default_bucket_and_prefix2-expected__with_user_input__with_default_bucket_only2]
FAILED tests/unit/test_estimator.py::test_stage_user_code_in_s3_default_bucket_and_prefix_combinations[None-None-expected__without_user_input__with_default_bucket_and_default_prefix3-expected__without_user_input__with_default_bucket_only3-expected__with_user_input__with_default_bucket_and_prefix3-expected__with_user_input__with_default_bucket_only3]
FAILED tests/unit/test_estimator.py::test_stage_user_code_in_s3_default_bucket_and_prefix_combinations[output_path4-s3://code-bucket/code-prefix/code-prefix2-expected__without_user_input__with_default_bucket_and_default_prefix4-expected__without_user_input__with_default_bucket_only4-expected__with_user_input__with_default_bucket_and_prefix4-expected__with_user_input__with_default_bucket_only4]
FAILED tests/unit/test_estimator.py::test_stage_user_code_in_s3_default_bucket_and_prefix_combinations[output_path5-None-expected__without_user_input__with_default_bucket_and_default_prefix5-expected__without_user_input__with_default_bucket_only5-expected__with_user_input__with_default_bucket_and_prefix5-expected__with_user_input__with_default_bucket_only5]
FAILED tests/unit/test_estimator.py::test_stage_user_code_in_s3_default_bucket_and_prefix_combinations[file://output-bucket/output-prefix/output-prefix2-s3://code-bucket/code-prefix/code-prefix2-expected__without_user_input__with_default_bucket_and_default_prefix6-expected__without_user_input__with_default_bucket_only6-expected__with_user_input__with_default_bucket_and_prefix6-expected__with_user_input__with_default_bucket_only6]
FAILED tests/unit/test_estimator.py::test_stage_user_code_in_s3_default_bucket_and_prefix_combinations[file://output-bucket/output-prefix/output-prefix2-None-expected__without_user_input__with_default_bucket_and_default_prefix7-expected__without_user_input__with_default_bucket_only7-expected__with_user_input__with_default_bucket_and_prefix7-expected__with_user_input__with_default_bucket_only7]

Update:

Realized these tests require local .aws configuration and confirmed they pass with local configuration setup.

src/sagemaker/estimator.py

vpbhargav · 2023-10-09T18:53:31Z

/bot run pr

src/sagemaker/estimator.py

tests/unit/test_estimator.py

sheilaliuxl · 2023-10-09T19:00:14Z

tests/unit/test_estimator.py

+}
+DISTRIBUTION_SM_TORCH_DIST_AND_DDP_DISABLED = {
+    "smdistributed": {"enabled": True},
+    "torch_distributed": {"enabled": True}


tests/unit/test_estimator.py

src/sagemaker/estimator.py

goelakash · 2023-10-09T21:56:43Z

/bot run all

goelakash · 2023-10-09T22:11:29Z

/bot run pr

goelakash · 2023-10-09T22:52:10Z

/bot run pr

knikure

/bot run all

sagemaker-bot · 2023-10-09T23:26:56Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: c213df5
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-09T23:54:25Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 861e2ba
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-10T00:45:06Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 861e2ba
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-17T23:24:13Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: db1a1cb
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-17T23:46:28Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: db1a1cb
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

knikure

/bot run slow-tests, local-mode-tests

sagemaker-bot · 2023-10-18T15:31:10Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: db1a1cb
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-18T15:31:28Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: db1a1cb
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-18T19:45:58Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 594b028
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-18T20:04:00Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 594b028
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-18T20:31:18Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 594b028
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-18T20:47:24Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 594b028
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-19T02:59:38Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 594b028
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…use CUDA 12.1 enabled DLC containers and/or p5 instance with smdistributed enabled and torch-distributed disabled.

…imator. Add additional unsupported image tests. Clean up tests.

… stated.

bhupendrasingh

/bot run all

sagemaker-bot · 2023-10-23T17:54:50Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 67e8813
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-23T18:05:04Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 67e8813
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-23T18:16:19Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 67e8813
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-23T18:29:00Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 67e8813
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-10-23T18:48:20Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 67e8813
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

knikure

/bot run pr

sagemaker-bot · 2023-10-23T22:00:16Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 67e8813
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

viclzhu requested a review from a team as a code owner September 19, 2023 19:09

viclzhu requested review from vpbhargav and removed request for a team September 19, 2023 19:09

viclzhu force-pushed the update-parameter-passing branch from 784510c to 6acd3c3 Compare September 19, 2023 19:10

rahul003 approved these changes Sep 19, 2023

View reviewed changes

knikure assigned vpbhargav Sep 20, 2023

viclzhu marked this pull request as draft October 4, 2023 17:55

viclzhu force-pushed the update-parameter-passing branch from 4582897 to 7332b6f Compare October 4, 2023 21:34

viclzhu marked this pull request as ready for review October 5, 2023 00:54

viclzhu force-pushed the update-parameter-passing branch 3 times, most recently from 3b081a5 to c936d1c Compare October 5, 2023 07:21

vpbhargav approved these changes Oct 5, 2023

View reviewed changes

jeet4320 suggested changes Oct 5, 2023

View reviewed changes

src/sagemaker/estimator.py Outdated Show resolved Hide resolved

viclzhu force-pushed the update-parameter-passing branch 2 times, most recently from 9098d5a to 8e2f08e Compare October 5, 2023 22:13

sheilaliuxl reviewed Oct 9, 2023

View reviewed changes

jeet4320 suggested changes Oct 9, 2023

View reviewed changes

src/sagemaker/estimator.py Outdated Show resolved Hide resolved

goelakash force-pushed the update-parameter-passing branch from ed3c5a0 to 7d67d89 Compare October 9, 2023 21:55

knikure reviewed Oct 9, 2023

View reviewed changes

goelakash force-pushed the update-parameter-passing branch from c213df5 to 861e2ba Compare October 9, 2023 23:49

knikure reviewed Oct 18, 2023

View reviewed changes

knikure self-assigned this Oct 18, 2023

viclzhu and others added 7 commits October 23, 2023 09:58

change: allow smdistributed to be enabled with torch_distributed.

463f112

deprecation: Blocking submission of distribution configs that try to …

36bfae8

…use CUDA 12.1 enabled DLC containers and/or p5 instance with smdistributed enabled and torch-distributed disabled.

change: Add 2.0-gpu-py310-cu121 to unsupported dlc images for the est…

20cd14b

…imator. Add additional unsupported image tests. Clean up tests.

fix: fix formatting

4dd0abb

fix: Allow distributions that don't have torch_distributed explicitly…

00d310e

… stated.

fix: fix formatting

4b86475

fix: Allowing instance_type to be ParameterString object.

67e8813

bhupendrasingh force-pushed the update-parameter-passing branch from 594b028 to 67e8813 Compare October 23, 2023 16:58

bhupendrasingh reviewed Oct 23, 2023

View reviewed changes

knikure approved these changes Oct 23, 2023

View reviewed changes

knikure unassigned vpbhargav Oct 23, 2023

knikure reviewed Oct 23, 2023

View reviewed changes

knikure merged commit 4e6f2e0 into aws:master Oct 23, 2023

change: allow smdistributed to be enabled with torch_distributed. #4129

change: allow smdistributed to be enabled with torch_distributed. #4129

Uh oh!

Conversation

viclzhu commented Sep 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

General

Tests

Uh oh!

viclzhu commented Sep 19, 2023

Uh oh!

rohithn1 commented Oct 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Update:

Uh oh!

Uh oh!

vpbhargav commented Oct 9, 2023

Uh oh!

Uh oh!

Uh oh!

sheilaliuxl Oct 9, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

goelakash commented Oct 9, 2023

Uh oh!

goelakash commented Oct 9, 2023

Uh oh!

goelakash commented Oct 9, 2023

Uh oh!

knikure left a comment

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Oct 9, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 9, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 10, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 17, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 17, 2023

AWS CodeBuild CI Report

Uh oh!

knikure left a comment

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 18, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 19, 2023

AWS CodeBuild CI Report

Uh oh!

bhupendrasingh left a comment

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Oct 23, 2023

AWS CodeBuild CI Report

Uh oh!

viclzhu commented Sep 19, 2023 •

edited

Loading

rohithn1 commented Oct 2, 2023 •

edited

Loading