infra: refactor toolkit tests. #56

nadiaya · 2020-03-13T00:37:49Z

Refactor toolkit tests from container tests:

Separate toolkit tests from container tests;
Remove test logic for other libraries installed in the container;
Provide options to build and push images as part of running tests.
Add PR build logic specific to just toolkit.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

sagemaker-bot · 2020-03-13T00:39:00Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-13T02:32:05Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-13T02:33:33Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-13T04:33:24Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-13T09:45:32Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

laurenyu · 2020-03-13T16:21:20Z

test-toolkit/conftest.py

@@ -1,4 +1,4 @@
-# Copyright 2019-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.


is this because you copied from pytorch (training)?

laurenyu · 2020-03-13T16:22:38Z

test-toolkit/conftest.py

    parser.addoption('--region', default='us-west-2')
-    parser.addoption('--framework-version', default=PyTorch.LATEST_VERSION)
+    parser.addoption('--framework-version', default="1.4.0")
    parser.addoption('--py-version', choices=['2', '3'], default=str(sys.version_info.major))


should we default to Python 3 since the Python 2 images weren't released?

laurenyu · 2020-03-13T16:22:50Z

test-toolkit/conftest.py

    parser.addoption('--instance-type')
-    parser.addoption('--docker-base-name', default='pytorch')
+    parser.addoption('--accelerator-type')
+    parser.addoption('--docker-base-name', default='sagemaker-pytorch-training')


laurenyu · 2020-03-13T16:23:37Z

test-toolkit/docker/1.4.0/Dockerfile.dlc.cpu

@@ -0,0 +1,10 @@
+ARG region
+from 763104351884.dkr.ecr.$region.amazonaws.com/pytorch-inference:1.4.0-cpu-py3


nit: capitalize "FROM"

test-toolkit/docker/1.4.0/Dockerfile.pytorch

laurenyu · 2020-03-13T16:26:34Z

test-toolkit/resources/mnist/mnist.py

@@ -191,9 +190,6 @@ def save_model(model, model_dir):


 if __name__ == '__main__':
-    # test opencv
-    print(cv.__version__)
-
    parser = argparse.ArgumentParser()


is all the training code in this file still needed?

test-toolkit/resources/mnist/model_cpu/mnist.py

laurenyu · 2020-03-13T16:28:11Z

test-toolkit/resources/mnist/model_eia/mnist.py

+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+# This file is intentionally left blank to utilize default_model_fn and default_predict_fn


I'd put a newline before line 13 just so that it's easier to tell there's a real comment after the license header. Also, maybe rename this file so that it's obvious it's empty.

laurenyu · 2020-03-13T16:29:01Z

test-toolkit/utils/__init__.py

@@ -1,4 +1,4 @@
-# Copyright 2019-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.


any particular reason why?

the file was copied :)

sagemaker-bot · 2020-03-13T18:20:48Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-13T20:20:12Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

laurenyu · 2020-03-13T21:04:29Z

test-toolkit/resources/mnist/model_cpu/mnist.py

-        test_loss, correct, len(test_loader.dataset),
-        100. * correct / len(test_loader.dataset)))
-
-
 def model_fn(model_dir):


do we actually need multiple copies now? (from a glance, the model_fns look the same across each "mnist" file?)

i don't know why they were created. it happened for an unknown to me reason during the image split.

laurenyu · 2020-03-13T21:05:12Z

test-toolkit/resources/mnist/model_cpu/mnist.py

@@ -1,4 +1,4 @@
-# Copyright 2019-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.


nit: no need to change the copyright year 😂

sagemaker-bot · 2020-03-13T22:28:41Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-14T01:59:24Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-14T05:30:26Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-16T11:36:51Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

laurenyu · 2020-03-16T22:40:05Z

test-toolkit/resources/mnist/model_cpu/call_model_fn_once.py

+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")
+print("HEY")


laurenyu · 2020-03-16T22:40:44Z

test-toolkit/resources/mnist/model_eia/mnist.py

+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+# This file is intentionally left blank to utilize default_model_fn and default_predict_fn


test-toolkit/integration/__init__.py

laurenyu · 2020-03-16T22:43:21Z

test-toolkit/integration/sagemaker/test_mnist.py

+        if accelerator_type is not None:
+            predictor = pytorch.deploy(initial_instance_count=1, instance_type=instance_type,
+                                       accelerator_type=accelerator_type, endpoint_name=endpoint_name)
+        else:
+            predictor = pytorch.deploy(initial_instance_count=1, instance_type=instance_type,
+                                       endpoint_name=endpoint_name)


I think deploy() should do the right thing with accelerator_type=None?

sagemaker-bot · 2020-03-17T01:14:32Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-17T07:32:50Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

infra: refactor toolkit tests.

00c8606

fix generic docker file.

4775f2f

nadiaya requested a review from laurenyu March 13, 2020 02:33

Move buildspec back.

9269730

nadiaya added 6 commits March 12, 2020 23:00

An attempt to fix flake8 error (couldn't repo locally).

71ec190

Decrease required code coverage :(.

c820be1

fix typo

395ae28

remove print statements.

855ca1e

Add missing tests.

45536f3

Add test coverage back

cd5c22c

fix test

04bd435

laurenyu reviewed Mar 13, 2020

View reviewed changes

nadiaya added 2 commits March 13, 2020 09:43

Remove training related code from test scripts.

26f6339

PR comments + adding an EIA test.

9030bff

fix flake8.

f4966e2

Do not build EIA image outside of test.

c11a16b

laurenyu reviewed Mar 13, 2020

View reviewed changes

nadiaya force-pushed the testref branch from 90317ec to 6e1e1a9 Compare March 16, 2020 09:43

laurenyu reviewed Mar 16, 2020

View reviewed changes

make sure we show that we use the scripts that we use.

dde94b9

nadiaya force-pushed the testref branch from 4ceac50 to dde94b9 Compare March 16, 2020 23:46

laurenyu approved these changes Mar 17, 2020

View reviewed changes

nadiaya merged commit e049d31 into master Mar 17, 2020

nadiaya deleted the testref branch March 17, 2020 18:08

		@@ -1,4 +1,4 @@
		# Copyright 2019-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
		# Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.

		@@ -0,0 +1,10 @@
		ARG region
		from 763104351884.dkr.ecr.$region.amazonaws.com/pytorch-inference:1.4.0-cpu-py3

		@@ -1,4 +1,4 @@
		# Copyright 2019-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
		# Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.

infra: refactor toolkit tests. #56

infra: refactor toolkit tests. #56

Uh oh!

Conversation

nadiaya commented Mar 13, 2020

Uh oh!

sagemaker-bot commented Mar 13, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 13, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 13, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 13, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 13, 2020

AWS CodeBuild CI Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Mar 13, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 13, 2020

AWS CodeBuild CI Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Mar 13, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 14, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 14, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 16, 2020

AWS CodeBuild CI Report

Uh oh!

Choose a reason for hiding this comment

👋

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Mar 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 17, 2020

AWS CodeBuild CI Report