SM_HPS is worth mentioning #522
Conversation
Just found a SM_HPS environmental variable that dumps the original hyperparameters as a json string. This helps preserve types and is definitely worth mentioning!
Codecov Report
@@ Coverage Diff @@
## master #522 +/- ##
=======================================
Coverage 92.78% 92.78%
=======================================
Files 71 71
Lines 5366 5366
=======================================
Hits 4979 4979
Misses 387 387Continue to review full report at Codecov.
|
| @@ -76,6 +78,9 @@ For example, a training script might start with the following: | |||
| parser.add_argument('--batch-size', type=int, default=100) | |||
| parser.add_argument('--learning-rate', type=float, default=0.1) | |||
|
|
|||
| # an alternative way to load hyperparameters via SM_HPS environmental variable. | |||
| parser.add_argument('--sm-hps', type=json.loads, default=os.environ['SM_HPS']) | |||
There was a problem hiding this comment.
parser.add_argument('--sm-hps', type=dict, default=json.loads(os.environ['SM_HPS']))
There was a problem hiding this comment.
Question: I thought type=json.loads because it is an actual json string. Would type=dict handle situations like nested dictionaries or lists?
There was a problem hiding this comment.
Sorry, misread your comment. Let me do some background check again.
There was a problem hiding this comment.
@icywang86rui I don't actually know this usage. What is the input format to work with type=dict?
I found that type=dict gave me errors, similar to this post: https://stackoverflow.com/questions/7625786/type-dict-in-argparse-add-argument
There was a problem hiding this comment.
when type=dict gave you errors, it was even with json.loads(os.environ['SM_HPS']) as the default? The Stack Overflow question seems to be about if you feed only a string to the parser when it's expecting a dict.
Thanks for the comments. Btw, `type=json.loads` seems to be the correct grammar (and `type=dict` causes errors). See example https://stackoverflow.com/questions/7625786/type-dict-in-argparse-add-argument
|
I don’t see how you can pass a dict as a command line argument.
…On Thu, Dec 20, 2018 at 8:37 AM Lauren Yu ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/sagemaker/mxnet/README.rst
<#522 (comment)>
:
> @@ -76,6 +78,9 @@ For example, a training script might start with the following:
parser.add_argument('--batch-size', type=int, default=100)
parser.add_argument('--learning-rate', type=float, default=0.1)
+ # an alternative way to load hyperparameters via SM_HPS environmental variable.
+ parser.add_argument('--sm-hps', type=json.loads, default=os.environ['SM_HPS'])
when type=dict gave you errors, it was even with
json.loads(os.environ['SM_HPS']) as the default? The Stack Overflow
question seems to be about if you feed only a string to the parser when
it's expecting a dict.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#522 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AIiQ6idHonI8sQM4PYpUZNNjHMiCEftQks5u67zagaJpZM4Y9h5K>
.
|
|
@yifeim yeah that was my thought too when reading the Stack Overflow question you linked to, but it seems weird that anyway, I think I'm fine with this PR - can you add a changelog entry? |
laurenyu
dismissed
icywang86rui’s stale review
see correspondence in this PR
Just found a SM_HPS environmental variable that dumps the original hyperparameters as a json string. This helps preserve types and is definitely worth mentioning!
Issue #, if available:
Description of changes:
Merge Checklist
Put an
xin 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.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.