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

consolidate Linux workflows on CPU and GPU #7189

Merged
merged 33 commits into from
Mar 1, 2023

Conversation

pmeier
Copy link
Contributor

@pmeier pmeier commented Feb 8, 2023

Spin off from #7149. Instead of having CPU and GPU test separately, we can have them in a single workflow as well. We should prefer that to avoid copy-pasting to much stuff where it is not necessary.

cc @seemethere

eval "$(${CONDA_PATH} shell.bash hook)"

echo '::group::Set PyTorch conda channel and wheel index'
# TODO: Can we maybe have this as environment variable in the job template? For example, `IS_RELEASE`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will open an issue in test-infra for that

Comment on lines +5 to +7
# Prepare conda
CONDA_PATH=$(which conda)
eval "$(${CONDA_PATH} shell.bash hook)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIUC, all the generic jobs come with conda pre-installed and use bash, right? If so, could we maybe run conda init before we enter the script? Otherwise stuff like conda activate doesn't work without running the highlighted commands before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least the conda part is not true for Windows. @osalpekar is this intentional?

Copy link
Member

Choose a reason for hiding this comment

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

It is not intentional - since this is the first application of the windows job, we'll need to identify and patch these gaps as we go :) This line would need to be added to the windows_job to ensure conda is installed: https://github.com/pytorch/test-infra/blob/main/.github/workflows/macos_job.yml#L84

.github/unittest.sh Outdated Show resolved Hide resolved
.github/unittest.sh Outdated Show resolved Hide resolved
@pmeier pmeier mentioned this pull request Feb 27, 2023
Comment on lines +5 to +7
# Prepare conda
CONDA_PATH=$(which conda)
eval "$(${CONDA_PATH} shell.bash hook)"
Copy link
Member

Choose a reason for hiding this comment

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

It is not intentional - since this is the first application of the windows job, we'll need to identify and patch these gaps as we go :) This line would need to be added to the windows_job to ensure conda is installed: https://github.com/pytorch/test-infra/blob/main/.github/workflows/macos_job.yml#L84

.github/workflows/test-linux.yml Show resolved Hide resolved
@pmeier pmeier merged commit caf12f8 into pytorch:main Mar 1, 2023
@pmeier pmeier deleted the consolidate-linux-ci branch March 1, 2023 21: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: D44416540

fbshipit-source-id: 13d9ced6c5723b1cf99d8a45a38683c09f51c533
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