Skip to content

EstimatorBase._json_encode_hyperparameters(hyperparams) breaks sagemaker_training.py #2949

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

Closed
osdf opened this issue Feb 22, 2022 · 1 comment

Comments

@osdf
Copy link

osdf commented Feb 22, 2022

Describe the bug

https://github.com/aws/sagemaker-training-toolkit/blob/master/src/sagemaker_training/logging_config.py#L29 needs an integer for level.
Through https://github.com/aws/sagemaker-python-sdk/blob/dev/src/sagemaker/estimator.py#L685 the value in hyperparams[CONTAINER_LOG_LEVEL_PARAM_NAME] (which is logging.INFO, the integer value 20) gets converted to a string ('20'), and hence https://github.com/aws/sagemaker-training-toolkit/blob/master/src/sagemaker_training/logging_config.py#L37 breaks.

Similarly (if one fixes the '20' issue by hand),
https://github.com/aws/sagemaker-training-toolkit/blob/master/src/sagemaker_training/files.py#L136 won't identify that code needs to be downloaded from s3, because the entry for https://github.com/aws/sagemaker-python-sdk/blob/dev/src/sagemaker/model.py#L65 is wrapped into a string, having the form "'s3://....'".

To reproduce
Run with script-mode, using newest sagemaker python sdk (2.76.0)

Expected behavior
It works.

System information
A description of your system. Please provide:

  • SageMaker Python SDK version: 2.76.0
  • Framework name: None
  • Framework version: None
  • Python version: 3.8
  • CPU or GPU: CPU and GPU
  • Custom Docker image (Y/N): Yes.

Additional context
Add any other context about the problem here.

@osdf osdf added the type: bug label Feb 22, 2022
osdf pushed a commit to osdf/sagemaker-python-sdk that referenced this issue Feb 22, 2022
Extend the set of types that are not handled by json.dumps to elementary types.
@osdf osdf mentioned this issue Feb 23, 2022
9 tasks
osdf pushed a commit to osdf/sagemaker-python-sdk that referenced this issue Mar 2, 2022
Hyperparameters are only json encoded at the end of setting up a Sagemaker job.
@navinsoni
Copy link
Contributor

Fixed by #3043

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

No branches or pull requests

4 participants