-
Notifications
You must be signed in to change notification settings - Fork 125
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
Fix logging issues #108
Fix logging issues #108
Conversation
Use asyncio to read stdout and stderr streams in realtime Report Exit code on failures Convey user informative message if process gets OOM Killed Filter out stderr to look for error messages and report Prepend tags to the log files to enable easy filtering in CloudWatch Update Amazon Licensing Update SM doc urls
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Support - Added Py38, Removed py36 and py27 Added unittests for asyncio APIs Fixed formatting issues - black and pylint Fix formatting
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@aws/aws-frameworks Could you guys please review this PR? |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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.
Small nit in dockerfile, but otherwise looks good. LGTM.
test/container/dummy/Dockerfile
Outdated
&& tar -xvf Python-$PYTHON_VERSION.tgz \ | ||
&& cd Python-$PYTHON_VERSION \ | ||
&& ./configure && make && make install \ | ||
&& make && make install && rm -rf ../Python-$PYTHON_VERSION* |
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.
Duplicate "make" and "make install" commands here, so it's building python twice.
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.
Fixed the duplicate make issue.
Agree with Joe, LGTM. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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 work on the PR !!!
Some minor questions, but looks good overall.
def create(cmd, error_class, cwd=None, capture_error=False, **kwargs): | ||
"""Spawn a process with subprocess.Popen for the given command. | ||
|
||
async def watch(stream, proc_per_host): |
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.
can this function error out ? What happens if it does? Should we try-except?
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.
This function can error out. The problem with using try-except
here is that this logic is being invoked from a try
and if there is an error.
run_async( |
@@ -33,4 +33,4 @@ phases: | |||
- cd ../.. | |||
|
|||
# run local integration tests | |||
- IGNORE_COVERAGE=- tox -e py36,py37 -- test/integration/local | |||
- IGNORE_COVERAGE=- tox -e py37,py38 -- test/integration/local |
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.
Are we planning not to update py36 containers?
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.
No. This is because of asyncio.run
which is available from Py3.7. There are other ways to achieve this using asyncio in py3.6 but it is too low level and handling them is very difficult.
def __init__(self, cmd, return_code=None, output=None): | ||
self.return_code = return_code | ||
def __init__(self, cmd, return_code=None, output=None, info=None): | ||
self.return_code = str(return_code) |
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.
can return_code
be None, in which case conversion to str
will fail. should we add a check here?
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.
str
conversion of NoneType
object will be just None. So it should be fine in this case.
self.cmd = cmd | ||
self.output = output | ||
self.extra_info = info | ||
super(_CalledProcessError, self).__init__() | ||
|
||
def __str__(self): | ||
if six.PY3 and self.output: |
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.
can we remove the PY3 check, since PY2 is deprecated ?
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.
Thanks for catching these. Removed PY2 related in the following commit
|
||
with patch.dict(os.environ, clear=True): | ||
os.environ["AWS_ACCESS_KEY_ID"] = "ABCD" |
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.
Why dummy key ID ? do we need to hash this or generate a random key ?
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.
This is just a dummy test where in I am mimicking AWS_ACCESS_KEY_ID
. The SMTT picks it from os.environ
and passes it to SM training job. In order to test this, I am patching the dict with this key, value pair.
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.
Addressed review comments.
def __init__(self, cmd, return_code=None, output=None): | ||
self.return_code = return_code | ||
def __init__(self, cmd, return_code=None, output=None, info=None): | ||
self.return_code = str(return_code) |
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.
str
conversion of NoneType
object will be just None. So it should be fine in this case.
self.cmd = cmd | ||
self.output = output | ||
self.extra_info = info | ||
super(_CalledProcessError, self).__init__() | ||
|
||
def __str__(self): | ||
if six.PY3 and self.output: |
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.
Thanks for catching these. Removed PY2 related in the following commit
def create(cmd, error_class, cwd=None, capture_error=False, **kwargs): | ||
"""Spawn a process with subprocess.Popen for the given command. | ||
|
||
async def watch(stream, proc_per_host): |
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.
This function can error out. The problem with using try-except
here is that this logic is being invoked from a try
and if there is an error.
run_async( |
|
||
with patch.dict(os.environ, clear=True): | ||
os.environ["AWS_ACCESS_KEY_ID"] = "ABCD" |
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.
This is just a dummy test where in I am mimicking AWS_ACCESS_KEY_ID
. The SMTT picks it from os.environ
and passes it to SM training job. In order to test this, I am patching the dict with this key, value pair.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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.
LGTM !
* fix: propagate log level to aws services (aws#79) * fix: propagate log level to aws services * drop py27 and add py38 support * update unit test * recover buildspeck * remove py38 build * install latest sagemaker 1.x version * fix: removing py27/py38 * fix arg name Co-authored-by: Chuyang Deng <[email protected]> * prepare release v3.6.3 * update development version to v3.6.4.dev0 * doc: fix typo in ENVIRONMENT_VARIABLES.md (aws#81) Removed typo ')'. Co-authored-by: Ajay Karpur <[email protected]> * prepare release v3.6.3.post0 * update development version to v3.6.4.dev0 * infra: use ECR-hosted image for ubuntu:16.04 (aws#87) * infra: use ECR-hosted image for ubuntu:16.04 * use public ECR repo * disable prompts in Docker build * fix: workaround to print stderr when capturing (aws#86) Co-authored-by: Ajay Karpur <[email protected]> * prepare release v3.6.4 * update development version to v3.6.5.dev0 * feature: add data parallelism support (aws#3) (aws#8) * change: use format in place of f-strings and use comment style type annotations (aws#10) * change: update tox to use sagemaker 2.18.0 for tests * prepare release v3.7.0 * update development version to v3.7.1.dev0 * fix:decode binary stderr string before dumping it out (aws#89) * fix:decode binary stderr string before dumping it out * fix failing test Co-authored-by: Rui Wang Napieralski <[email protected]> * prepare release v3.7.1 * update development version to v3.7.2.dev0 * change: set btl_vader_single_copy_mechanism to none (aws#90) * prepare release v3.7.2 * update development version to v3.7.3.dev0 * change: set btl_vader_single_copy_mechanism to none to avoid Read -1 Warning messages (aws#95) * prepare release v3.7.3 * update development version to v3.7.4.dev0 * Update Dockerfile to accomomdate Rust dependency. (aws#98) * Update Dockerfile to accomomdate Rust dependency. cryptography module has added RUST as its dependency. Upgrading PIP to solve this dependency. * pinning to particular version of pip pinned to pip version 21.0.1 which solves the Rust dependency * prepare release v3.7.4 * update development version to v3.7.5.dev0 * Change: smdataparallel change FI_PROVIDER to efa from sockets (aws#96) * prepare release v3.7.5 * update development version to v3.7.6.dev0 * feature: smdataparallel custom mpi options support (aws#99) * feature: smdataparallel custom mpi options support * Fixed pylint * Fixed black-check * Fixed unit test * prepare release v3.8.0 * update development version to v3.8.1.dev0 * feature: smdataparallel enable EFA RDMA flag (aws#101) * feature: smdataparallel enable EFA RDMA flag * added changes to unit test * updated the flag to use only for ml.p4d.24xlarge instance * prepare release v3.9.0 * update development version to v3.9.1.dev0 * change: [smdataparallel] better messages to establish the SSH connection between workers (aws#103) * change: [smdataparallel] better messages for to establish the SSH connection between workers * python timeout.timeout raises TimeoutError * Added detailed error message * prepare release v3.9.1 * update development version to v3.9.2.dev0 * Reverted -x FI_EFA_USE_DEVICE_RDMA=1 to fix a crash on PyTorch Dataloaders for Distributed training (aws#106) * prepare release v3.9.2 * update development version to v3.9.3.dev0 * Fix logging issues (aws#108) * Fix logging issues Use asyncio to read stdout and stderr streams in realtime Report Exit code on failures Convey user informative message if process gets OOM Killed Filter out stderr to look for error messages and report Prepend tags to the log files to enable easy filtering in CloudWatch Update Amazon Licensing Update SM doc urls Support - Added Py38, Removed py36 and py27 Added unittests for asyncio APIs Install libssl1.1 and openssl packages * prepare release v3.9.3 * update development version to v3.9.4.dev0 * breaking: Add py38, dropped py36 and py2 support. Bump pypi to 4.0.0 (changes from PR aws#108) (aws#109) * prepare release v4.0.0 * update development version to v4.0.1.dev0 * Fix: Enable custom failure logging (aws#118) * prepare release v4.0.1 * update development version to v4.0.2.dev0 * feature: add back FI_EFA_USE_DEVICE_RDMA=1 flag, revert 2936f22 (aws#121) fix: fixed the black lint, upgraded black to version 21.3.0 fix: remove u prefix of strings, as python3 defaults to unicode strings note: EFA is only available on p3dn or p4dn instances note: EFA version 1.15.1 and OFI 1.1.5-aws have the issue fixed note: black format reference on remove u prefix https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#strings * prepare release v4.1.0 * update development version to v4.1.1.dev0 * fix: missing args when shell script is used (aws#122) * prepare release v4.1.1 * update development version to v4.1.2.dev0 Co-authored-by: Chuyang <[email protected]> Co-authored-by: Chuyang Deng <[email protected]> Co-authored-by: ci <ci> Co-authored-by: Pedro Martins <[email protected]> Co-authored-by: Ajay Karpur <[email protected]> Co-authored-by: sboshin <[email protected]> Co-authored-by: ChaiBapchya <[email protected]> Co-authored-by: Dan <[email protected]> Co-authored-by: icywang86rui <[email protected]> Co-authored-by: Rui Wang Napieralski <[email protected]> Co-authored-by: Eric Johnson <[email protected]> Co-authored-by: Karan Jariwala <[email protected]> Co-authored-by: Rajan Singh <[email protected]> Co-authored-by: Piyush Ghai <[email protected]> Co-authored-by: Daiming Yang <[email protected]>
* fix: propagate log level to aws services (aws#79) * fix: propagate log level to aws services * drop py27 and add py38 support * update unit test * recover buildspeck * remove py38 build * install latest sagemaker 1.x version * fix: removing py27/py38 * fix arg name Co-authored-by: Chuyang Deng <[email protected]> * prepare release v3.6.3 * update development version to v3.6.4.dev0 * doc: fix typo in ENVIRONMENT_VARIABLES.md (aws#81) Removed typo ')'. Co-authored-by: Ajay Karpur <[email protected]> * prepare release v3.6.3.post0 * update development version to v3.6.4.dev0 * infra: use ECR-hosted image for ubuntu:16.04 (aws#87) * infra: use ECR-hosted image for ubuntu:16.04 * use public ECR repo * disable prompts in Docker build * fix: workaround to print stderr when capturing (aws#86) Co-authored-by: Ajay Karpur <[email protected]> * prepare release v3.6.4 * update development version to v3.6.5.dev0 * feature: add data parallelism support (aws#3) (aws#8) * change: use format in place of f-strings and use comment style type annotations (aws#10) * change: update tox to use sagemaker 2.18.0 for tests * prepare release v3.7.0 * update development version to v3.7.1.dev0 * fix:decode binary stderr string before dumping it out (aws#89) * fix:decode binary stderr string before dumping it out * fix failing test Co-authored-by: Rui Wang Napieralski <[email protected]> * prepare release v3.7.1 * update development version to v3.7.2.dev0 * change: set btl_vader_single_copy_mechanism to none (aws#90) * prepare release v3.7.2 * update development version to v3.7.3.dev0 * change: set btl_vader_single_copy_mechanism to none to avoid Read -1 Warning messages (aws#95) * prepare release v3.7.3 * update development version to v3.7.4.dev0 * Update Dockerfile to accomomdate Rust dependency. (aws#98) * Update Dockerfile to accomomdate Rust dependency. cryptography module has added RUST as its dependency. Upgrading PIP to solve this dependency. * pinning to particular version of pip pinned to pip version 21.0.1 which solves the Rust dependency * prepare release v3.7.4 * update development version to v3.7.5.dev0 * Change: smdataparallel change FI_PROVIDER to efa from sockets (aws#96) * prepare release v3.7.5 * update development version to v3.7.6.dev0 * feature: smdataparallel custom mpi options support (aws#99) * feature: smdataparallel custom mpi options support * Fixed pylint * Fixed black-check * Fixed unit test * prepare release v3.8.0 * update development version to v3.8.1.dev0 * feature: smdataparallel enable EFA RDMA flag (aws#101) * feature: smdataparallel enable EFA RDMA flag * added changes to unit test * updated the flag to use only for ml.p4d.24xlarge instance * prepare release v3.9.0 * update development version to v3.9.1.dev0 * change: [smdataparallel] better messages to establish the SSH connection between workers (aws#103) * change: [smdataparallel] better messages for to establish the SSH connection between workers * python timeout.timeout raises TimeoutError * Added detailed error message * prepare release v3.9.1 * update development version to v3.9.2.dev0 * Reverted -x FI_EFA_USE_DEVICE_RDMA=1 to fix a crash on PyTorch Dataloaders for Distributed training (aws#106) * prepare release v3.9.2 * update development version to v3.9.3.dev0 * Fix logging issues (aws#108) * Fix logging issues Use asyncio to read stdout and stderr streams in realtime Report Exit code on failures Convey user informative message if process gets OOM Killed Filter out stderr to look for error messages and report Prepend tags to the log files to enable easy filtering in CloudWatch Update Amazon Licensing Update SM doc urls Support - Added Py38, Removed py36 and py27 Added unittests for asyncio APIs Install libssl1.1 and openssl packages * prepare release v3.9.3 * update development version to v3.9.4.dev0 * breaking: Add py38, dropped py36 and py2 support. Bump pypi to 4.0.0 (changes from PR aws#108) (aws#109) * prepare release v4.0.0 * update development version to v4.0.1.dev0 * Fix: Enable custom failure logging (aws#118) * prepare release v4.0.1 * update development version to v4.0.2.dev0 * feature: add back FI_EFA_USE_DEVICE_RDMA=1 flag, revert 2936f22 (aws#121) fix: fixed the black lint, upgraded black to version 21.3.0 fix: remove u prefix of strings, as python3 defaults to unicode strings note: EFA is only available on p3dn or p4dn instances note: EFA version 1.15.1 and OFI 1.1.5-aws have the issue fixed note: black format reference on remove u prefix https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#strings * prepare release v4.1.0 * update development version to v4.1.1.dev0 * fix: missing args when shell script is used (aws#122) * prepare release v4.1.1 * update development version to v4.1.2.dev0 * fix: fix flaky issue with incorrect rc being given (aws#124) * fix: fix flaky issue with incorrect rc being given * Add logging around proc.wait. * prepare release v4.1.2 * update development version to v4.1.3.dev0 * Feature: Adding new parameter for TF Multi Worker Mirrored Strategy (aws#130) * feature: Adding new parameter for TF Multi Worker Mirrored Strategy * fix: changing variable name for MWMS * fix: freezing protobuf version and renaming variable for MWMS * fix: linting * prepare release v4.1.3 * update development version to v4.1.4.dev0 Co-authored-by: Chuyang <[email protected]> Co-authored-by: Chuyang Deng <[email protected]> Co-authored-by: ci <ci> Co-authored-by: Pedro Martins <[email protected]> Co-authored-by: Ajay Karpur <[email protected]> Co-authored-by: sboshin <[email protected]> Co-authored-by: ChaiBapchya <[email protected]> Co-authored-by: Dan <[email protected]> Co-authored-by: icywang86rui <[email protected]> Co-authored-by: Rui Wang Napieralski <[email protected]> Co-authored-by: Eric Johnson <[email protected]> Co-authored-by: Karan Jariwala <[email protected]> Co-authored-by: Rajan Singh <[email protected]> Co-authored-by: Piyush Ghai <[email protected]> Co-authored-by: Daiming Yang <[email protected]> Co-authored-by: matherit <[email protected]> Co-authored-by: Loki <[email protected]>
* fix: propagate log level to aws services (aws#79) * fix: propagate log level to aws services * drop py27 and add py38 support * update unit test * recover buildspeck * remove py38 build * install latest sagemaker 1.x version * fix: removing py27/py38 * fix arg name Co-authored-by: Chuyang Deng <[email protected]> * prepare release v3.6.3 * update development version to v3.6.4.dev0 * doc: fix typo in ENVIRONMENT_VARIABLES.md (aws#81) Removed typo ')'. Co-authored-by: Ajay Karpur <[email protected]> * prepare release v3.6.3.post0 * update development version to v3.6.4.dev0 * infra: use ECR-hosted image for ubuntu:16.04 (aws#87) * infra: use ECR-hosted image for ubuntu:16.04 * use public ECR repo * disable prompts in Docker build * fix: workaround to print stderr when capturing (aws#86) Co-authored-by: Ajay Karpur <[email protected]> * prepare release v3.6.4 * update development version to v3.6.5.dev0 * feature: add data parallelism support (aws#3) (aws#8) * change: use format in place of f-strings and use comment style type annotations (aws#10) * change: update tox to use sagemaker 2.18.0 for tests * prepare release v3.7.0 * update development version to v3.7.1.dev0 * fix:decode binary stderr string before dumping it out (aws#89) * fix:decode binary stderr string before dumping it out * fix failing test Co-authored-by: Rui Wang Napieralski <[email protected]> * prepare release v3.7.1 * update development version to v3.7.2.dev0 * change: set btl_vader_single_copy_mechanism to none (aws#90) * prepare release v3.7.2 * update development version to v3.7.3.dev0 * change: set btl_vader_single_copy_mechanism to none to avoid Read -1 Warning messages (aws#95) * prepare release v3.7.3 * update development version to v3.7.4.dev0 * Update Dockerfile to accomomdate Rust dependency. (aws#98) * Update Dockerfile to accomomdate Rust dependency. cryptography module has added RUST as its dependency. Upgrading PIP to solve this dependency. * pinning to particular version of pip pinned to pip version 21.0.1 which solves the Rust dependency * prepare release v3.7.4 * update development version to v3.7.5.dev0 * Change: smdataparallel change FI_PROVIDER to efa from sockets (aws#96) * prepare release v3.7.5 * update development version to v3.7.6.dev0 * feature: smdataparallel custom mpi options support (aws#99) * feature: smdataparallel custom mpi options support * Fixed pylint * Fixed black-check * Fixed unit test * prepare release v3.8.0 * update development version to v3.8.1.dev0 * feature: smdataparallel enable EFA RDMA flag (aws#101) * feature: smdataparallel enable EFA RDMA flag * added changes to unit test * updated the flag to use only for ml.p4d.24xlarge instance * prepare release v3.9.0 * update development version to v3.9.1.dev0 * change: [smdataparallel] better messages to establish the SSH connection between workers (aws#103) * change: [smdataparallel] better messages for to establish the SSH connection between workers * python timeout.timeout raises TimeoutError * Added detailed error message * prepare release v3.9.1 * update development version to v3.9.2.dev0 * Reverted -x FI_EFA_USE_DEVICE_RDMA=1 to fix a crash on PyTorch Dataloaders for Distributed training (aws#106) * prepare release v3.9.2 * update development version to v3.9.3.dev0 * Fix logging issues (aws#108) * Fix logging issues Use asyncio to read stdout and stderr streams in realtime Report Exit code on failures Convey user informative message if process gets OOM Killed Filter out stderr to look for error messages and report Prepend tags to the log files to enable easy filtering in CloudWatch Update Amazon Licensing Update SM doc urls Support - Added Py38, Removed py36 and py27 Added unittests for asyncio APIs Install libssl1.1 and openssl packages * prepare release v3.9.3 * update development version to v3.9.4.dev0 * breaking: Add py38, dropped py36 and py2 support. Bump pypi to 4.0.0 (changes from PR aws#108) (aws#109) * prepare release v4.0.0 * update development version to v4.0.1.dev0 * Fix: Enable custom failure logging (aws#118) * prepare release v4.0.1 * update development version to v4.0.2.dev0 * feature: add back FI_EFA_USE_DEVICE_RDMA=1 flag, revert 2936f22 (aws#121) fix: fixed the black lint, upgraded black to version 21.3.0 fix: remove u prefix of strings, as python3 defaults to unicode strings note: EFA is only available on p3dn or p4dn instances note: EFA version 1.15.1 and OFI 1.1.5-aws have the issue fixed note: black format reference on remove u prefix https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#strings * prepare release v4.1.0 * update development version to v4.1.1.dev0 * fix: missing args when shell script is used (aws#122) * prepare release v4.1.1 * update development version to v4.1.2.dev0 * fix: fix flaky issue with incorrect rc being given (aws#124) * fix: fix flaky issue with incorrect rc being given * Add logging around proc.wait. * prepare release v4.1.2 * update development version to v4.1.3.dev0 * Feature: Adding new parameter for TF Multi Worker Mirrored Strategy (aws#130) * feature: Adding new parameter for TF Multi Worker Mirrored Strategy * fix: changing variable name for MWMS * fix: freezing protobuf version and renaming variable for MWMS * fix: linting * prepare release v4.1.3 * update development version to v4.1.4.dev0 * Use framework provided error class and stack trace as error message (aws#123) * log smddp exceptions * update exception class * clean up error msg * address comments * Add Error Categorization for SMMP * add pytorch errors for SMMP && minor fixes * feature: allow framework libraries to supply exceptions to track and report as failure reason. Added support for SMDDP and SMMP custom exceptions. Include custom exception as error class and de-duplicated stack trace as error message. Added tests for wacthing single, list of exceptions and also support existing internal exceptions. Co-authored-by: haohanchen-yagao <[email protected]> Co-authored-by: Joe Evans <[email protected]> * prepare release v4.1.4 * update development version to v4.1.5.dev0 * Fix none exception class issue for mpi (aws#131) * fix: Fix none exception class issue for mpi * Add unit test for SMP exception import * reformat * fix import format * rename get exception function * prepare release v4.1.5 * update development version to v4.1.6.dev0 * update: protobuf version to overlap with TF requirements (aws#134) * update: protobuf version to overlap with TF requirements * fix: upper bound * prepare release v4.1.6 * update development version to v4.1.7.dev0 * feature: Heterogeneous cluster changes (aws#135) * prepare release v4.2.0 * update development version to v4.2.1.dev0 * fix: handle utf-8 decoding exceptions while processing stdout and stderr streams * prepare release v4.2.1 * update development version to v4.2.2.dev0 * fix: specify flake8 config explicitly (aws#138) * change: update distribution_instance_group for pytorch ddp * fix: Removed version hardcoding for sagemaker test dependency (aws#141) * prepare release v4.2.2 * update development version to v4.2.3.dev0 * change: update num_processes_per_host for smdataparallel runner * prepare release v4.2.3 * update development version to v4.2.4.dev0 * Feature: Create a new distribution mechanism for PT-XLA (aws#137) * Create a new distribution mechanism for PT-XLA * Adding new unit tests targetting PT-XLA distributed training * Reformatting according to guidelines * Linting changes * Linting changes * Linting changes * Test Mock syntax fix * Test Mock syntax fix * Fixing syntax error * Fixing syntax error * Revert "Fixing syntax error" This reverts commit 48a10c5. * Fixing syntax error * Fixing syntax error * + new test to target the PT-XLA Distributed runner * + new test to target the PT-XLA Distributed runner * + new test to target the PT-XLA Distributed runner * + new test to target the PT-XLA Distributed runner * + new test to target the PT-XLA Distributed runner * + new test to target the PT-XLA Distributed runner * + new test to target the PT-XLA Distributed runner * Add verbose reporting for tox tests * Fixing syntax errors * Fixing syntax errors * Fixing syntax errors * Fixing syntax errors * Adding more tests targeting PT-XLA DT mechanism * edits for flake8 * edits for black * fixing test errors * fixing test errors * fixing test errors * fixing test errors * fixing test errors * fixing container build for unit testing * fixing container build for unit testing * retry tests * fixing container build for unit testing * fixing container build for unit testing * fixing container execution for unit testing * fixing container execution for unit testing * Refactoring some tests as integration tests * Refactoring some tests as integration tests * Refactoring some tests as integration tests * Refactoring some tests as integration tests * Refactoring some tests as integration tests * Removing stale files * Removing stale test container * Fix: adding EFA specific setup to distributed training runner for PT-XLA (aws#143) * fix: adding EFA specific setup to distributed training runner for PT-XLA * test: testing new env variables for PT-XLA on EFA * prepare release v4.2.4 * update development version to v4.2.5.dev0 * relax exception type (aws#140) no qa * prepare release v4.2.5 * update development version to v4.2.6.dev0 * fix: Enable PT XLA distributed training on homogeneous clusters (aws#144) * fix: adding bypass for PT XLA distributed training on homogeneous cluster * fix: linting * prepare release v4.2.6 * update development version to v4.2.7.dev0 * fix: improve worker node wait logic and update EFA flags (aws#145) * prepare release v4.2.7 * update development version to v4.2.8.dev0 * Fix: Args for worker nodes in smdataparallel jobs (aws#147) * fix worker args for sm dataparallel jobs * prepare release v4.2.8 * update development version to v4.2.9.dev0 * Fix merge conflicts Co-authored-by: Chuyang <[email protected]> Co-authored-by: Chuyang Deng <[email protected]> Co-authored-by: ci <ci> Co-authored-by: Pedro Martins <[email protected]> Co-authored-by: Ajay Karpur <[email protected]> Co-authored-by: sboshin <[email protected]> Co-authored-by: ChaiBapchya <[email protected]> Co-authored-by: Dan <[email protected]> Co-authored-by: icywang86rui <[email protected]> Co-authored-by: Rui Wang Napieralski <[email protected]> Co-authored-by: Eric Johnson <[email protected]> Co-authored-by: Karan Jariwala <[email protected]> Co-authored-by: Rajan Singh <[email protected]> Co-authored-by: Piyush Ghai <[email protected]> Co-authored-by: Daiming Yang <[email protected]> Co-authored-by: matherit <[email protected]> Co-authored-by: Loki <[email protected]> Co-authored-by: Lai Wei <[email protected]> Co-authored-by: haohanchen-yagao <[email protected]> Co-authored-by: Joe Evans <[email protected]> Co-authored-by: haohanchen-yagao <[email protected]> Co-authored-by: Nishanth Hegde <[email protected]> Co-authored-by: Vishwa Karia <[email protected]> Co-authored-by: Nishanth Hegde <[email protected]> Co-authored-by: Jihyeong Lee <[email protected]> Co-authored-by: Loki <[email protected]>
Issue #, if available:
Description of changes:
This PR addresses the logging issues while Framework training jobs on SageMaker:
Testing done:
Tests pass with "tox -e py37,py38 tests/unit"
Merge Checklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.