Support of Horovod and TF 1.12 for TensorFlow Script Mode. TFS 1.12 support #567
Conversation
Codecov Report
@@ Coverage Diff @@
## master #567 +/- ##
=========================================
+ Coverage 92.69% 92.7% +0.01%
=========================================
Files 71 71
Lines 5405 5418 +13
=========================================
+ Hits 5010 5023 +13
Misses 395 395
Continue to review full report at Codecov.
|
src/sagemaker/tensorflow/defaults.py
Outdated
| @@ -12,4 +12,4 @@ | |||
| # language governing permissions and limitations under the License. | |||
| from __future__ import absolute_import | |||
|
|
|||
| TF_VERSION = '1.11' | |||
There was a problem hiding this comment.
Don't change default version.
Add LATEST_VERSION to the estimator instead, e.g.: https://github.com/aws/sagemaker-python-sdk/blob/master/src/sagemaker/mxnet/estimator.py#L33
There was a problem hiding this comment.
For Mxnet also the default version in defaults.py and LATEST_VERSION in estimator.py is explicityly defined.
MxNet estimatopr.py => https://github.com/aws/sagemaker-python-sdk/blob/master/src/sagemaker/mxnet/estimator.py#L33
MxNet defaults.py -> https://github.com/aws/sagemaker-python-sdk/blob/master/src/sagemaker/mxnet/defaults.py
There was a problem hiding this comment.
Don't change the version @uditbhatia . Version is a required field starting by TF 1.12.
There was a problem hiding this comment.
I didnt changed any field
| (default: None). Currently we only support distributed training with parameter servers. To enable it | ||
| use the following setup: | ||
| (default: None). Currently we support distributed training with parameter servers and MPI. To enable | ||
| parameter servers use the following setup: |
There was a problem hiding this comment.
Follow up:
we should have a test that checks parmater server + mpi works.
src/sagemaker/tensorflow/defaults.py
Outdated
| @@ -12,4 +12,4 @@ | |||
| # language governing permissions and limitations under the License. | |||
| from __future__ import absolute_import | |||
|
|
|||
| TF_VERSION = '1.11' | |||
There was a problem hiding this comment.
Don't change the version @uditbhatia . Version is a required field starting by TF 1.12.
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.