-
Notifications
You must be signed in to change notification settings - Fork 7.1k
[Nova] Migrate Linux CPU job to Generic Job #6797
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good, just a few optimizations to make, and to clean up some un-needed jobs
.github/workflows/test-linux-cpu.yml
Outdated
@@ -13,58 +13,50 @@ env: | |||
CHANNEL: "nightly" | |||
|
|||
jobs: | |||
setup: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this setup
job?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I didn't see the repository input for the generic job. Yeah we can use that instead.
.github/workflows/test-linux-cpu.yml
Outdated
export CUDATOOLKIT="cpuonly" | ||
|
||
# Set CHANNEL | ||
if [[${{ (github.event_name == 'pull_request' && startsWith(github.base_ref, 'release')) || startsWith(github.ref, 'refs/heads/release') }} ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should actually be available as regular environment variables so it might be better to use those instead, see: https://docs.github.com/en/actions/learn-github-actions/environment-variables
.github/workflows/test-linux-cpu.yml
Outdated
|
||
# Create Conda Env | ||
conda create -yp "${ENV_NAME}" python="${PYTHON_VERSION}" numpy libpng jpeg scipy | ||
export CONDA_RUN="conda run -p ${ENV_NAME}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You actually don't necessarily need to do this, you can just do a conda activate
and run regular conda commands in this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
043b6d8
to
d23c434
Compare
Hey @osalpekar! 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 |
Summary: * [Nova] Migrate Linux CPU job to Generic Job * branch ref for composite action job * move checkout step to separate job * added runs-on * nit fixes * no need to run conda sheel script thing * Channel is set inside the script * add remaining env vars * nit env var fix * cleanup * simplify unneeded jobs * name of the conda env should be the path * remove main ref to use PR Reviewed By: YosuaMichael Differential Revision: D40588164 fbshipit-source-id: 45aad3577f19edc8296bc0b97435ad510d46a436
Migrating the Linux CPU Testing job to the Generic Linux Job format that will be used long-term for CI jobs.