Add unittest job to CircleCI #2328

guyang3532 · 2020-06-18T02:35:46Z

Remove unittest from conda build, and add them as independent unittest jobs.
So unittests will not triggered at every conda build and test in an independent test env.
test at #2332

peterjc123

Generally LGTM, please remember to remove the windows legacy build jobs and its job spec.

codecov · 2020-06-18T14:43:08Z

Codecov Report

Merging #2328 into master will increase coverage by 2.24%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2328      +/-   ##
==========================================
+ Coverage   68.49%   70.73%   +2.24%     
==========================================
  Files          93       93              
  Lines        7655     7706      +51     
  Branches     1177     1177              
==========================================
+ Hits         5243     5451     +208     
+ Misses       2075     1903     -172     
- Partials      337      352      +15

Impacted Files	Coverage Δ
torchvision/models/detection/transform.py	`95.55% <0.00%> (+0.02%)`	⬆️
torchvision/ops/poolers.py	`97.02% <0.00%> (+0.02%)`	⬆️
torchvision/models/inception.py	`84.64% <0.00%> (+0.05%)`	⬆️
torchvision/models/googlenet.py	`82.32% <0.00%> (+0.09%)`	⬆️
torchvision/models/detection/roi_heads.py	`82.34% <0.00%> (+0.11%)`	⬆️
torchvision/models/detection/generalized_rcnn.py	`88.33% <0.00%> (+0.19%)`	⬆️
torchvision/models/detection/_utils.py	`67.34% <0.00%> (+0.22%)`	⬆️
torchvision/datasets/imagenet.py	`82.45% <0.00%> (+0.31%)`	⬆️
torchvision/ops/boxes.py	`84.44% <0.00%> (+0.35%)`	⬆️
torchvision/models/densenet.py	`82.81% <0.00%> (+0.41%)`	⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 446eac6...80c34fd. Read the comment docs.

guyang3532 · 2020-06-18T15:00:34Z

Generally LGTM, please remember to remove the windows legacy build jobs and its job spec.

thank you, they have been removed.

fmassa

Thanks for the PR!

I think that we might now be missing our single CUDA CI that was effectively running CUDA tests, which was torchvision_linux_py3.8_cu102_cuda . So we would need to add back a CUDA CI test before we can merge this, otherwise we won't be able to check for CUDA correctness (now it only compiles torchvision, but doesn't run any tests).

@seemethere could you also have a look?

fmassa · 2020-06-19T09:32:49Z

.circleci/unittest/linux/scripts/run_test.sh

+conda activate ./env
+
+python -m torch.utils.collect_env
+pytest --cov=torchaudio --junitxml=test-results/junit.xml -v --durations 20 test


nit: torchaudio

fmassa · 2020-06-19T09:33:18Z

.circleci/unittest/linux/scripts/setup_env.sh

+# This script is for setting up environment in which unit test is ran.
+# To speed up the CI time, the resulting environment is cached.
+#
+# Do not install PyTorch and torchaudio here, otherwise they also get cached.


nit: torchaudio

fmassa · 2020-06-19T09:34:55Z

.circleci/unittest/windows/scripts/setup_env.sh

+# This script is for setting up environment in which unit test is ran.
+# To speed up the CI time, the resulting environment is cached.
+#
+# Do not install PyTorch and torchaudio here, otherwise they also get cached.


nit: torchaudio

malfet · 2020-06-19T15:56:24Z

.circleci/config.yml.in

+  unittest_linux_cpu:
+    <<: *binary_common
+    docker:
+      - image: "pytorch/manylinux-cuda102"


Is there a reason why cuda image is used to run cpu tests?

When I added the original version of this to torchaudio, I could not find CPU version. If you know what's an appropriate image for CPU, let me know, I will also update on torchaudio side.

malfet · 2020-06-19T16:00:26Z

.circleci/config.yml.in

+      image: ubuntu-1604-cuda-10.1:201909-23
+    resource_class: gpu.small
+    environment:
+      image_name: "pytorch/manylinux-cuda101"


Is there a reason why you prefer CUDA 10.1 vs CUDA 10.2

malfet · 2020-06-19T16:01:06Z

.circleci/config.yml.in

+    executor:
+      name: windows-gpu
+    environment:
+      CUDA_VERSION: "10.1"


Same as above: why CUDA 10.1 rather than CUDA 10.2?

We use CUDA 10.1 before this change just because it is already installed in the image. If you want, we may do it in a follow-up PR.

seemethere · 2020-06-19T16:37:13Z

.circleci/config.yml.in

+    <<: *binary_common
+    machine:
+      image: ubuntu-1604-cuda-10.1:201909-23
+    resource_class: gpu.small


Aren't this job and the unittest_linux_cpu basically the same job?

Should we just consolidate them and then add an extra parameter to designate resource_class?

@seemethere When I worked on this, I could not find a Docker runner with GPU. My understanding is that for GPU, CircleCI gives Virtual Machine, so we need to run the same script with docker run argument. While non-GPU environment CIrcleCI gives is Docker container.
Let me know if you know a way to request GPU Docker environment (not VM).

seemethere · 2020-06-19T16:37:39Z

.circleci/config.yml.in

+          path: test-results
+
+  unittest_windows_gpu:
+    <<: *binary_common


Same comment here about consolidating build jobs

peterjc123 · 2020-06-22T09:14:35Z

Thanks for the PR!

I think that we might now be missing our single CUDA CI that was effectively running CUDA tests, which was torchvision_linux_py3.8_cu102_cuda . So we would need to add back a CUDA CI test before we can merge this, otherwise we won't be able to check for CUDA correctness (now it only compiles torchvision, but doesn't run any tests).

@seemethere could you also have a look?

Do you mean to run unit tests with CUDA? There is already one named unittest_linux_gpu that is testing with CUDA 10.1. Or you just want to add one for CUDA 10.2? I don't think we should run tests through conda-build anymore. It's just a waste of time.

fmassa · 2020-06-22T09:54:12Z

@peterjc123

There is already one named unittest_linux_gpu that is testing with CUDA 10.1.

I might be missing something, but the CI is not running any test named unittest_*_gpu I believe?

peterjc123 · 2020-06-22T12:52:58Z

@fmassa They run only on master. Maybe we should make at least one of them run in PRs, right?

fmassa · 2020-06-22T13:29:00Z

They run only on master. Maybe we should make at least one of them run in PRs, right?

Yes, they should run in PRs as well. I see that it has been updated now, thanks!

fmassa · 2020-06-22T13:30:28Z

Maybe we don't need all CUDA tests running on every PR (would be expensive?), but only a small subset -- before we only had one CI running CUDA tests on Linux and Windows, maybe we could keep the same here?

cc @seemethere for thoughts

guyang3532 · 2020-06-23T02:55:26Z

Maybe we don't need all CUDA tests running on every PR (would be expensive?), but only a small subset -- before we only had one CI running CUDA tests on Linux and Windows, maybe we could keep the same here?

cc @seemethere for thoughts

if we only put unittest_gpu_py3.8 on pr and others on master?

fmassa · 2020-06-23T09:57:47Z

@guyang3532

if we only put unittest_gpu_py3.8 on pr and others on master?

That sounds good to me, although I would also put unittest_windwos_gpu_py3.8 on every PR as well

fmassa

Changes look good to me, thanks a lot!

I think we can cleanup one more build which is not needed anymore, but otherwise this seems ok to me.

I'll let @seemethere merge this one, as he is maintaining CI and releases for torchvision.

fmassa · 2020-06-26T09:09:05Z

.circleci/config.yml.in

      - binary_linux_conda_cuda:
          name: torchvision_linux_py3.8_cu102_cuda
          python_version: "3.8"
          cu_version: "cu102"


This build is I believe not needed anymore, and the corresponding binary_linux_conda_cuda is not needed anymore I think.

guyang3532 force-pushed the add_win_ci_unittest branch 7 times, most recently from fc8b338 to 374d0c6 Compare June 18, 2020 07:49

guyang3532 changed the title ~~Add unittest to Windows CI~~ Add unittest job to CircleCI Jun 18, 2020

peterjc123 approved these changes Jun 18, 2020

View reviewed changes

peterjc123 requested review from seemethere and fmassa June 18, 2020 14:00

guyang3532 force-pushed the add_win_ci_unittest branch from 374d0c6 to dad5f10 Compare June 18, 2020 14:24

guyang3532 mentioned this pull request Jun 18, 2020

[test]test unittest on CI #2332

Closed

fmassa requested changes Jun 19, 2020

View reviewed changes

malfet reviewed Jun 19, 2020

View reviewed changes

seemethere reviewed Jun 19, 2020

View reviewed changes

guyang3532 force-pushed the add_win_ci_unittest branch 3 times, most recently from 61ef663 to 87eff97 Compare June 22, 2020 07:21

guyang3532 force-pushed the add_win_ci_unittest branch from 87eff97 to 68583dd Compare June 22, 2020 13:25

guyang3532 force-pushed the add_win_ci_unittest branch from 68583dd to afa3105 Compare June 24, 2020 14:09

fmassa approved these changes Jun 26, 2020

View reviewed changes

Add unittest to CI

80c34fd

guyang3532 force-pushed the add_win_ci_unittest branch from afa3105 to 80c34fd Compare June 30, 2020 12:38

seemethere approved these changes Jun 30, 2020

View reviewed changes

seemethere merged commit 9d9e716 into pytorch:master Jun 30, 2020

fmassa mentioned this pull request Jun 30, 2020

PR: Add libpng and libjpeg-turbo requirement into conda recipe #2301

Merged

de-vri-es pushed a commit to fizyr-forks/torchvision that referenced this pull request Aug 4, 2020

Add unittest job to CircleCI (pytorch#2328)

0e54ccf

fmassa mentioned this pull request Sep 29, 2020

Add macos unittest #2727

Merged

datumbox mentioned this pull request Dec 19, 2020

Speedup InceptionV3 and GoogleNet tests on windows (upgrade scipy) #3196

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unittest job to CircleCI #2328

Add unittest job to CircleCI #2328

guyang3532 commented Jun 18, 2020 •

edited

peterjc123 left a comment

codecov bot commented Jun 18, 2020 •

edited

guyang3532 commented Jun 18, 2020 •

edited

fmassa left a comment

fmassa Jun 19, 2020

fmassa Jun 19, 2020

fmassa Jun 19, 2020

malfet Jun 19, 2020

mthrok Jun 19, 2020

malfet Jun 19, 2020

malfet Jun 19, 2020

peterjc123 Jun 22, 2020

seemethere Jun 19, 2020

mthrok Jun 19, 2020

seemethere Jun 19, 2020

peterjc123 commented Jun 22, 2020

fmassa commented Jun 22, 2020

peterjc123 commented Jun 22, 2020 •

edited

fmassa commented Jun 22, 2020

fmassa commented Jun 22, 2020

guyang3532 commented Jun 23, 2020

fmassa commented Jun 23, 2020

fmassa left a comment

fmassa Jun 26, 2020

Add unittest job to CircleCI #2328

Add unittest job to CircleCI #2328

Conversation

guyang3532 commented Jun 18, 2020 • edited

peterjc123 left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 18, 2020 • edited

Codecov Report

guyang3532 commented Jun 18, 2020 • edited

fmassa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterjc123 commented Jun 22, 2020

fmassa commented Jun 22, 2020

peterjc123 commented Jun 22, 2020 • edited

fmassa commented Jun 22, 2020

fmassa commented Jun 22, 2020

guyang3532 commented Jun 23, 2020

fmassa commented Jun 23, 2020

fmassa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guyang3532 commented Jun 18, 2020 •

edited

codecov bot commented Jun 18, 2020 •

edited

guyang3532 commented Jun 18, 2020 •

edited

peterjc123 commented Jun 22, 2020 •

edited