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

Skip CPU-only tests on CircleCI machines with GPU #4002

Merged
merged 20 commits into from
Jun 9, 2021

Conversation

NicolasHug
Copy link
Member

This PR makes the CircleCI machines with GPU skip the tests that don't actually need a GPU. These tests are already extensively tested in the other CPU-only workflows.

@NicolasHug NicolasHug marked this pull request as draft June 8, 2021 14:05
@NicolasHug
Copy link
Member Author

This seems to be working as expected, see e.g. the linux gpu unittest workfow where the test_classification_model[cpu... tests are skipped while the CUDA ones are run. test_generalizedrcnn_transform_repr is also properly skipped.

The tests aren't properly skipped on Windows because of #4006, but we can merge this anyway.

Note: There are still a lot of CPU-only tests that are run on the GPU CI. These are all the tests in tests files like test_transforms.py that don't include GPU tests at all. I will open another PR to deal with these.

@NicolasHug NicolasHug marked this pull request as ready for review June 8, 2021 17:12
- run:
name: Install torchvision
command: docker run -t --gpus all -v $PWD:$PWD -w $PWD -e UPLOAD_CHANNEL -e CU_VERSION "${image_name}" .circleci/unittest/linux/scripts/install.sh
- run:
name: Run tests
command: docker run -e CIRCLECI -t --gpus all -v $PWD:$PWD -w $PWD "${image_name}" .circleci/unittest/linux/scripts/run_test.sh
command: docker run --env-file ./env.list --gpus all -v $PWD:$PWD -w $PWD "${image_name}" .circleci/unittest/linux/scripts/run_test.sh
Copy link
Member Author

@NicolasHug NicolasHug Jun 8, 2021

Choose a reason for hiding this comment

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

The previous -e CIRCLECI was actually enough for what we need here, but I feel like creating the file is more future proof (we might need more env variables in the future) and most importantly it allows to properly document why we need this, which can be useful if we start using docker for more than just linux.

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

With this PR we seem to have saved ~3min from the GPU CI, nice!

@NicolasHug NicolasHug merged commit c52a33a into pytorch:master Jun 9, 2021
@datumbox
Copy link
Contributor

datumbox commented Jun 9, 2021

@NicolasHug On unittest_windows_gpu_py3.8, I see cuda tests being skipped. Example:

test/test_ops.py::TestDeformConv::test_forward[0-True-cpu] PASSED        [ 96%]
test/test_ops.py::TestDeformConv::test_forward[0-True-cuda] SKIPPED      [ 96%]
test/test_ops.py::TestDeformConv::test_forward[0-False-cpu] PASSED       [ 96%]
test/test_ops.py::TestDeformConv::test_forward[0-False-cuda] SKIPPED     [ 96%]
test/test_ops.py::TestDeformConv::test_forward[33-True-cpu] PASSED       [ 96%]
test/test_ops.py::TestDeformConv::test_forward[33-True-cuda] SKIPPED     [ 96%]
test/test_ops.py::TestDeformConv::test_forward[33-False-cpu] PASSED      [ 96%]
test/test_ops.py::TestDeformConv::test_forward[33-False-cuda] SKIPPED    [ 96%]

Is this related to the temporary CicleCI driver issue for win GPU?

facebook-github-bot pushed a commit that referenced this pull request Jun 14, 2021
Reviewed By: fmassa

Differential Revision: D29097716

fbshipit-source-id: c4247acaf9f6c8b87eba4479e212891befb3efc8
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.

4 participants