Upgrade to TensorFlow 1.13.1 #184
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
* Add 1.13.1 dockerfiles
* Lower process number for one of the horovod local mode test
Too many processes running in parallel causes the test fail sometimes.
icywang86rui
changed the title
icywang86rui
changed the title
laurenyu
reviewed
May 8, 2019
docker/1.13.1/Dockerfile.cpu
Outdated
| ldconfig && \ | ||
| rm -rf /tmp/openmpi | ||
|
|
||
|
|
There was a problem hiding this comment.
don't need two empty newlines
docker/1.13.1/Dockerfile.cpu
Outdated
| rm -rf /tmp/openmpi | ||
|
|
||
|
|
||
| # Create a wrapper for OpenMPI to allow running as root by default |
There was a problem hiding this comment.
extra space before the pound sign
docker/1.13.1/Dockerfile.cpu
Outdated
| python get-pip.py --disable-pip-version-check --no-cache-dir "pip==18.1" && \ | ||
| rm get-pip.py | ||
|
|
||
| ARG framework_installable |
There was a problem hiding this comment.
can you group the args together?
docker/1.13.1/Dockerfile.cpu
Outdated
| markdown \ | ||
| tensorboard | ||
|
|
||
| ENV SAGEMAKER_TRAINING_MODULE sagemaker_tensorflow_container.training:main |
There was a problem hiding this comment.
add a newline to the end of the file
setup.py
Outdated
| @@ -53,7 +53,7 @@ def read(fname): | |||
| 'pandas', 'Pillow', 'h5py'], | |||
| extras_require={ | |||
| 'test': ['tox', 'flake8', 'pytest', 'pytest-cov', 'pytest-xdist', 'mock', | |||
| 'sagemaker>=1.15.2', 'tensorflow', 'docker-compose'], | |||
| 'sagemaker>=1.19.1', 'tensorflow', 'docker-compose'], | |||
There was a problem hiding this comment.
let's pin this to reduce the blast radius of potential issues with Python SDK releases
laurenyu
approved these changes
May 8, 2019
goswamig
reviewed
May 11, 2019
|
|
||
| RUN pip install --no-cache-dir -U \ | ||
| keras==2.2.4 \ | ||
| mpi4py==3.0.1 \ |
There was a problem hiding this comment.
why do you need this when you have openmpi ?
goswamig
reviewed
May 13, 2019
|
|
||
| RUN pip install --no-cache-dir -U \ | ||
| keras==2.2.4 \ | ||
| mpi4py==3.0.1 \ |
mirocody
reviewed
May 13, 2019
| # Install Open MPI | ||
| RUN mkdir /tmp/openmpi && \ | ||
| cd /tmp/openmpi && \ | ||
| curl -fSsL -O https://www.open-mpi.org/software/ompi/v3.1/downloads/openmpi-3.1.2.tar.gz && \ |
| curl \ | ||
| libcudnn7=7.4.1.5-1+cuda10.0 \ | ||
| # TensorFlow doesn't require libnccl anymore but Open MPI still depends on it | ||
| libnccl2 \ |
There was a problem hiding this comment.
As this is the dependencies of OpenMPI, and openmpi is pinned with version, could you please pin this version as well?
| libcudnn7=7.4.1.5-1+cuda10.0 \ | ||
| # TensorFlow doesn't require libnccl anymore but Open MPI still depends on it | ||
| libnccl2 \ | ||
| libnccl-dev \ |
|
Can you add a This is so that non-master nodes don't immediately exit if they come up before master starts. This is a bigger risk with jobs with many instances. |
Elizaaaaa
pushed a commit
to Elizaaaaa/sagemaker-tensorflow-container
that referenced
this pull request
Nov 4, 2019
* Upgrade to TensorFlow 1.13.1
* Add 1.13.1 dockerfiles
* Lower process number for one of the horovod local mode test
Too many processes running in parallel causes the test fail sometimes.
* Use multiprocessing.Process to start ps
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.
Too many processes running in parallel causes the test fail sometimes.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.