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

remove Linux GPU unittest from CircleCI #7354

Merged
merged 3 commits into from
Mar 1, 2023

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Feb 27, 2023

They were ported in #6804. There were some teething issues, but they have been running smoothly since at least one month now (#6957 (comment)).

cc @seemethere

@@ -722,96 +722,6 @@ jobs:
conda activate python${PYTHON_VERSION}
python -c "import torchvision"

unittest_linux_cpu:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was forgotten in the PR that ported the CPU tests. Since unittest_linux_cpu was not listed in the unittest group (only visible in config.yml, but not in this template), this job wasn't run.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks Philip, I'll stamp, but let's wait from someone from nova before merging.

Just to confirm, are we sure that the removed GPU job is fully covered by GA already (in terms of Python version, etc)? And are we also sure that removing the CPU unittest job isn't going to affect the doc build (I remember discussing it with you recently)

@pmeier
Copy link
Collaborator Author

pmeier commented Feb 27, 2023

Just to confirm, are we sure that the removed GPU job is fully covered by GA already (in terms of Python version, etc)?

Nope, coverage is not exactly the same:

vision/.circleci/config.yml

Lines 1344 to 1365 in cd33246

- unittest_linux_gpu:
cu_version: cu117
name: unittest_linux_gpu_py3.8
python_version: '3.8'
- unittest_linux_gpu:
cu_version: cu117
filters:
branches:
only:
- main
- nightly
name: unittest_linux_gpu_py3.9
python_version: '3.9'
- unittest_linux_gpu:
cu_version: cu117
filters:
branches:
only:
- main
- nightly
name: unittest_linux_gpu_py3.10
python_version: '3.10'

matrix:
python_version: ["3.8"]
cuda_arch_version: ["11.7"]

For both we test Python 3.8, but on CircleCI we also test 3.9 and 3.10 on the main and nightly branch, but not on PRs or even the release branches. I think this is intentional, but let's wait for Nova on that.

And are we also sure that removing the CPU unittest job isn't going to affect the doc build (I remember discussing it with you recently)

Yes, I'm sure. I was wrong back then. The docs workflow depends the build workflow

vision/.circleci/config.yml

Lines 1302 to 1312 in cd33246

- build_docs:
filters:
branches:
only:
- /.*/
tags:
only: /v[0-9]+(\.[0-9]+)*-rc[0-9]+/
name: build_docs
python_version: '3.8'
requires:
- binary_linux_wheel_py3.8_cpu

And that one is untouched by this PR. On main

$ grep unittest_linux_cpu .circleci/config.yml | wc -l
1

Meaning, this job is only defined, but never used. For that, it needs to show up in any group at the bottom of the file. Preferably here

unittest:

Copy link
Member

@osalpekar osalpekar left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@pmeier
Copy link
Collaborator Author

pmeier commented Mar 1, 2023

I've opened #7374 to keep track of handling the "main only" workflows later.

@pmeier pmeier merged commit e3da44b into pytorch:main Mar 1, 2023
@pmeier pmeier deleted the disable-linux-circleci branch March 1, 2023 19:23
@github-actions
Copy link

github-actions bot commented Mar 1, 2023

Hey @pmeier!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

facebook-github-bot pushed a commit that referenced this pull request Mar 30, 2023
Reviewed By: vmoens

Differential Revision: D44416634

fbshipit-source-id: 3646166c5334aee6bbe3a7695183a9c371922a73
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