Skip to content
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

test: create a test suite for examples #597

Merged
merged 6 commits into from
May 29, 2020
Merged

Conversation

yoavz
Copy link
Contributor

@yoavz yoavz commented May 28, 2020

Description

This PR creates a test suite associated with examples. It runs a minimal test on each example using --local --test mode. It is slightly more heavyweight than our unit tests since every example runs in a separate Python process, but more portable than full integration tests since no Determined cluster is required.

Run-time on my local macbook pro is 1 minute 42 seconds:

41.38s call     test_examples.py::test_official[official/cifar10_cnn_tf_keras-official/cifar10_cnn_tf_keras/const.yaml]
22.36s call     test_examples.py::test_official[official/cifar10_cnn_pytorch-official/cifar10_cnn_pytorch/const.yaml]
9.46s call     test_examples.py::test_official[official/multiple_lr_schedulers_pytorch-official/multiple_lr_schedulers_pytorch/const.yaml]
9.39s call     test_examples.py::test_official[official/mnist_pytorch-official/mnist_pytorch/const.yaml]
8.35s call     test_examples.py::test_official[official/mnist_estimator-official/mnist_estimator/const.yaml]
6.80s call     test_examples.py::test_official[official/fashion_mnist_tf_keras-official/fashion_mnist_tf_keras/const.yaml]
4.51s call     test_examples.py::test_official[official/iris_tf_keras-official/iris_tf_keras/const.yaml]

The runtime can likely further be improved by further optimizing --local --test mode with https://determinedai.atlassian.net/browse/DET-2931

DET-3110
DET-2952

Test Plan

N/A

Commentary (optional)

I had initially aimed to set-up these as unit tests running sequentially in a single Python process with test_one_batch. However, I hit some problems with conflicting module namespaces in examples -- for example, fashion_mnist_tf_keras and mnist_pytorch have a data.py module they import in their entrypoint module with import data. When executed sequentially in a single Python process, the second time import data is called it still uses the initial data module and fails. I figured it would be easier to execute each model definition in a separate process instead of removing this restriction or hacking the Python module namespace in some way.

@cla-bot cla-bot bot added the cla-signed label May 28, 2020
@yoavz yoavz changed the title test: add minimal tests to all official examples test: create a test suite for examples May 28, 2020
@yoavz yoavz requested a review from brainhart May 28, 2020 16:20
@brainhart
Copy link
Contributor

@yoavz let's add this to CircleCI?

@yoavz
Copy link
Contributor Author

yoavz commented May 28, 2020

@brain-good this is an initial pass at having "unit" tests for examples, with the caveat that I wasn't quite able to get them running all in the same Python process.

Let me know if this approach looks good to you and I'll add the Circle CI target and remove the redundant tests in e2e_tests.

examples/test_examples.py Outdated Show resolved Hide resolved
examples/test_examples.py Outdated Show resolved Hide resolved
@brainhart
Copy link
Contributor

Sounds good @yoavz , think the approach makes sense 👍

@yoavz yoavz force-pushed the det-3110 branch 5 times, most recently from a78e1a6 to 894a4d5 Compare May 28, 2020 17:27
e2e_tests/tests/experiment/test_tf_keras.py Outdated Show resolved Hide resolved
e2e_tests/tests/experiment/test_tf_estimator.py Outdated Show resolved Hide resolved
e2e_tests/tests/experiment/test_pytorch.py Outdated Show resolved Hide resolved
Copy link
Contributor

@brainhart brainhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good stuff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants