-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add support for Chainer 5.0 #491
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #491 +/- ##
==========================================
+ Coverage 94.23% 94.24% +<.01%
==========================================
Files 58 58
Lines 4444 4445 +1
==========================================
+ Hits 4188 4189 +1
Misses 256 256
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
talked offline - needs to make sure warnings and docs are updated appropriately
src/sagemaker/chainer/README.rst
Outdated
+-----------------------------+-------------+-------------+-------------+ | ||
| chainermn | 1.2.0 | 1.3.0 | N/A | | ||
+-----------------------------+-------------+-------------+-------------+ | ||
| CUDA (GPU image only) | 9.0 | 9.0 | 9.0 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you have extra leading spaces in some of these cells
src/sagemaker/chainer/README.rst
Outdated
|
||
The Docker images extend Ubuntu 16.04. | ||
|
||
You can select version of Chainer by passing a framework_version keyword arg to the Chainer Estimator constructor. | ||
You must select a version of Chainer by passing a framework_version keyword arg to the Chainer Estimator constructor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double backticks around framework_version
minor version, which will cause your training script to be run on the latest supported patch version of that minor | ||
version. | ||
You must select a version of Chainer by passing a ``framework_version`` keyword arg to the Chainer Estimator | ||
constructor. Currently supported versions are listed in the above table. You can also set framework_version to only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: add backticks around framework_version
Issue #, if available:
Description of changes:
Merge Checklist
Put an
x
in 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.