-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Hyperparameter values forcefully converted to strings, thus unable to pass a list #613
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
Comments
Hi @mmamaev , Although the list is encoded to string, you can decode them back to list when using it. For example, if you are using SageMaker's framework container, you can add the decode logic in the user script before using the list. So my question is, could you provide more details on this issue:
More details will help us understand the issue and may impact our decision whether to provide instruction for this case or make new feature as what you suggested (make the list automatically decoded in the hyperparameter json file). Thanks! |
Good morning, @yangaws, Yes, I am using my own container. No, I do not find it difficult to decode a string back to list... if I know that I have to do it. The problem is that sagemaker changes the type of the passed value when it is not expected to do so. The sagemaker doc reads:
Indeed, it mentions that the return type is dict[str, str]. However, this notice of the return type is present only for Moreover, the
So one is ok to assume that this is a "dictionary to use for training". Errors that can be introduced could be quite nasty. For example, the list My understanding is that you're being compliant to the following specification:
Forcing hyperparameter values to be strings looks rather unwarranted. I am interested what is the reason behind it. To work around, I have to implement additional checks to differentiate between reading "conventional" json and "sagemaker-style" json, doing additional decoding for the latter. I see this as an unnecessary complication. If this restriction cannot be lifted, I suggest that
|
Hi @mmamaev , Got what you mean. We need to talk whether this can be a feature change in our backlog (we make the hyperparameter as list in the json). But anyway our doc for now needs to be updated, let me add a doc label for now. Thanks for the explanation. |
Hey! This is something that has being a pain for us too.
Yes. The right solution to adapt ourselves to sagemaker's contract with hyperparameters is: keeping track of all possible key/values and their types in a custom image, just to parse it back to their expected format. And undoing the This is not easy to do when we have to send dozens of hyperparameters. Also, it is quite strange to send something like
and getting inside of the container something like:
Is there any chance of changing how sagemaker deals with hyperparameters? I imagine that the decision of forcing an |
we are facing this issue as well. And in our case it is nested dict (we use our customer docker images). you guys also convert double quotes to single quotes, so to effectively get this to work, i have to do this
|
Hi team, for those still having trouble here, I've been using these two functions to parse out the hyperparameters in my own estimator containers (it also strips leading 0's, just in case you work with coordinates in your day to day, which can cause trouble when trying to cast to an int or float):
It uses |
+1 I wasted a whole day of my life trying to debug this...training the docker container worked locally, but on sagemaker it did not! Truly an insidious issue. |
This issue is also being a pain for us too. Hopefully you can provide soon a way to handle lists as hyperparams |
I'm having the issue too. From what I gathered, there has been no official response to this thread for more than a year now, quite baffling. Furthermore, trying to find a workaround to @mmamaev 's situation, I found something maybe more insidious. This is the script, named
On my laptop, when I run When I run the following on sagemaker it fails:
where the image
Here is the end of the logs of the execution:
As you see, here So my workaround does not work and I have to rely on a painful hyperparameter decoding. @ihoelscher did a great job explaining why this should not be. Any news @yangaws ? Edit: in OP's case the following parameter decoding makes the code works (with
|
What is the status on this issue? I believe the relevant changes were not included on v2.0.0. |
@diegodebrito no, it wasn't included in v2.0.0 unfortunately cc @ajaykarpur, who might know about the current status of this issue |
Glad to create a PR for this immediately. I can't believe this has not been addressed in nearly a year. |
Hi Currently I'm doing https://docs.aws.amazon.com/sagemaker/latest/dg/ex1-train-model.html I'm in: "Step 4: Train a Model" And in the "3. Set the hyperparameters for the XGBoost algorithm by calling the set_hyperparameters method of the estimator." I'm having a problem after executing: After going through some documentation: https://xgboost.readthedocs.io/en/latest/parameter.html I see that the list of available options in This is how I have it right now
I've tried to use 2:
3:
The error output varies but it's always like this:
Do you guys now if I'm doing something wrong? |
@xjwalker
|
Nothing? |
As a workaround the environment variable This works much nicer than trying to parse the hyperparams from strings:
|
Would be great if this could get sorted or if the docs were more expressive |
bump |
any updates? parsing values can be a pain if it's all nested and has different data types. This adds a lot of extra workload. If user simply passes in a python dict, they would expect to get the same thru a simple load using the json module. |
I found a workaround when Instead of using Hope that helps! |
Parsing this was particularly annoying when passing a nested dict as a hyperparameter. One workaround I found that bypasses any manual parsing was to base64 encode the dict as a json string, and pass that to my Estimator and then decode in my training script: def b64encoded_str(j):
return base64.b64encode(json.dumps(j).encode('utf-8')).decode('utf-8')
config = {...} #Nest dict of lists, float, and dict
hyperparameters = {'b64config': b64encoded_str(config)}
estimator = PyTorch(
image_uri=image_uri
...,
hyperparamters=hyperparameters
)
estimator.fit(inputs=inputs) and then in my training script: def train(config):
....
def b64decode_str(s):
return json.loads(base64.b64decode(s).decode('utf-8'))
if __name__ == '__main__':
parser = argparse.ArgumentParser()
parser.add_argument('--b64config', type=str, required=True)
args = parser.parse_args()
cfg = b64decode_str(args.b64config)
train(cfg) Unideal that the hyperparameters are not legible on the Sagemaker console, but makes up for it in that it doesn't require any parsing! |
Have been 3 years. Since this came up. Nothing? |
The ModelTrainer class resolves this issue. This new ModelTrainer class offers enhanced functionality and improved user experience for model training workflows. Due to these significant improvements, ModelTrainer is now recommended as the standard approach for model training operations. |
I want ro pass a list as a hyperparameter to an estimator. For example,
hyperparameter key = 'a'
hyperparameter value = ['def', 'xyz']
This type of value is accepted and get verified:
I expect that sagemaker renders hyperparameters to the following json to be found inside /opt/ml/input/config/hyperparameters.json:
Instead, the contents of /opt/ml/input/config/hyperparameters.json is:
The
list
type of the value is lost, being forcefully converted to a string by this code:sagemaker-python-sdk/src/sagemaker/estimator.py
Lines 553 to 554 in 1c94079
The text was updated successfully, but these errors were encountered: