Add keras_model_fn integ test #338
Conversation
| script_path = os.path.join(DATA_DIR, 'cifar_10', 'source') | ||
| dataset_path = os.path.join(DATA_DIR, 'cifar_10', 'data') | ||
|
|
||
| with timeout(minutes=45): |
There was a problem hiding this comment.
Is it necessary to have a 45 minutes run for these test? Can we reduce the training steps and evaluation steps?
There was a problem hiding this comment.
Yep we need this because it currently times out frequently due to hosting. And we took their advice of using 45 mins.
The current timeout for tf cifar is also 45 mins https://github.com/aws/sagemaker-python-sdk/blob/master/tests/integ/test_tf_cifar.py#L38
And related PR is here: #337
There was a problem hiding this comment.
And I made the change to reduce training and evaluation steps too. Now it uses 500 and 5 which are same as what tf cifar test uses.
Codecov Report
@@ Coverage Diff @@
## master #338 +/- ##
======================================
Coverage 93.1% 93.1%
======================================
Files 51 51
Lines 3552 3552
======================================
Hits 3307 3307
Misses 245 245Continue to review full report at Codecov.
|
Issue #, if available:
Description of changes:
Add keras_model_fn integ test adapted from aws/amazon-sagemaker-examples#353
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.