Add distributed training support #98
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Launch user specified number of parameter servers * Add integ tests * Add unit tests * Add distributed sagemaker integ test
laurenyu
reviewed
Oct 26, 2018
|
|
||
| def _build_tf_config(hosts, current_host, ps_num=0, ps_task=False): | ||
| """ | ||
| Builds a dictionary containing cluster information based on number of hosts and number of |
There was a problem hiding this comment.
nit: combine the """ with the first line
| hosts (list[str]): List of host name in the cluster | ||
| current_host (str): Current host name | ||
| ps_num (int): Number of parameter servers | ||
| ps_task (bool): Set to True if this config is built for a ps server process |
There was a problem hiding this comment.
let's note the default for these
| ps = hosts[:ps_num] if len(hosts) > 1 and ps_num > 0 else None | ||
|
|
||
| def host_addresses(hosts, port=2222): | ||
| return ["{}:{}".format(host, port) for host in hosts] |
There was a problem hiding this comment.
use single quotes for strings
| return tf_config | ||
|
|
||
|
|
||
| def _get_env_vars_with_tf_config(env, ps_task): |
There was a problem hiding this comment.
I think prefixing with "get" is considered a little unpythonic or whatever - anyway, you can make it just _env_vars_with_tf_config
mvsusp
suggested changes
Oct 26, 2018
|
@mvsusp I will add the docstrings and double check the coverage test setup. Will document the container sizes as well. The integ tests only takes a few minutes. |
laurenyu
reviewed
Oct 29, 2018
laurenyu
reviewed
Oct 30, 2018
mvsusp
suggested changes
Oct 31, 2018
| && rm $framework_installable_local \ | ||
| && rm $framework_support_installable_local | ||
|
|
||
| ENV SAGEMAKER_TRAINING_MODULE sagemaker_tensorflow_container.training:main |
There was a problem hiding this comment.
You can use a linter like https://github.com/hadolint to help you here.
There was a problem hiding this comment.
I have run this locally, only thing comes up now is the pin version stuff. I will add it to the build in the next pr.
If parameter server is enabled we are going to launch the parameter server process on all the hosts. This chagne also includes some tests fixes.
laurenyu
reviewed
Nov 5, 2018
mvsusp
approved these changes
Nov 5, 2018
Elizaaaaa
pushed a commit
to Elizaaaaa/sagemaker-tensorflow-container
that referenced
this pull request
Nov 4, 2019
* Implement distributed support * Launch parameter server if user set sagemaker_parameter_server_enabled to be True * Add integ tests * Add unit tests * Add distributed sagemaker integ test * Add 1.11.0 and modify Dockerfile to reduce image size
Elizaaaaa
pushed a commit
to Elizaaaaa/sagemaker-tensorflow-container
that referenced
this pull request
Nov 4, 2019
* Implement distributed support * Launch parameter server if user set sagemaker_parameter_server_enabled to be True * Add integ tests * Add unit tests * Add distributed sagemaker integ test * Add 1.11.0 and modify Dockerfile to reduce image size
Elizaaaaa
pushed a commit
to Elizaaaaa/sagemaker-tensorflow-container
that referenced
this pull request
Nov 4, 2019
* Implement distributed support * Launch parameter server if user set sagemaker_parameter_server_enabled to be True * Add integ tests * Add unit tests * Add distributed sagemaker integ test * Add 1.11.0 and modify Dockerfile to reduce image size
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
This change sets up parameter servers for users in distributed training context. We read wether user wants to use parameter servers from hyperparameter 'sagemaker_parameter_server_enabled'. Use that to construct TF_CONFIG and launch ps and worker processes.
Compressed docker image sizes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.