Migrate conda, manywheel and libtorch docker builds to pytorch/pytorch #129022

juliagmt-google · 2024-06-19T01:15:30Z

Migration of Docker conda builds to pytorch/pytorch from pytorch/builder: https://github.com/pytorch/builder/blob/main/.github/workflows/build-conda-images.yml

Related to: pytorch/builder#1849

Migrate scripts and worklfows, adds logic to execute on PR and upload to ecr with github hash tag in order to test Docker build and nightly on PR.

Test when executing on PR, upload to ecr:
https://github.com/pytorch/pytorch/actions/runs/9799439218/job/27059691327

308535385114.dkr.ecr.us-east-1.amazonaws.com/pytorch/conda-builder-cpu:789cf8fcd738088860056160f6e9ea7cd005972b

Test With-Push, upload to dockerhub:
https://github.com/pytorch/pytorch/actions/runs/9799783407/job/27060633427

docker.io/pytorch/conda-builder:cpu done

Will upload here: https://hub.docker.com/r/pytorch/conda-builder/

Test using ecr image in the nightly workflow:
https://github.com/pytorch/pytorch/actions/runs/9798428933/job/27057835235#step:16:87

Note: This is first part that will build docker and upload it to either dockerhub or ecr. After merging followup PR will need to change conda nightly workflows to either use ecr image or dockerhub image, depending if we are running it on PR or from main/release branch.

Cleanup of workflows and scripts from builder repo: pytorch/builder#1923

pytorch-bot · 2024-06-19T01:15:33Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/129022

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure

As of commit 608e533 with merge base 637ab85 ():

NEW FAILURE - The following job has failed:

Lint / pr-sanity-checks (gh)
Process completed with exit code 1.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

malfet

Such PR could be landed only if it references PR against the builder that deletes the same files this one is migrating. Correct me if I'm wrong, but in its current form its impossible, as all the common files are used by both conda and manywheel Docker builds. If those two workflows are so intertwined, let's migrate all of them in one go.

Nits:

.ci/docker sounds like a wrong location for those files, as they are for use for deployment, rather that testing, so please consider different location.
I don't see how this change translates into the workflow that @atalman described, where one can test docker changes before merging the PR (I assume it would require some changes to the binary builds workflow, wouldn't it?)

atalman · 2024-07-08T13:21:02Z

Such PR could be landed only if it references PR against the builder that deletes the same files this one is migrating. Correct me if I'm wrong, but in its current form its impossible, as all the common files are used by both conda and manywheel Docker builds. If those two workflows are so intertwined, let's migrate all of them in one go.

Sounds good. Let me bring everything in.

Nits:

.ci/docker sounds like a wrong location for those files, as they are for use for deployment, rather that testing, so please consider different location.

Perhaps .cd/docker would be better in this case ?

I don't see how this change translates into the workflow that @atalman described, where one can test docker changes before merging the PR (I assume it would require some changes to the binary builds workflow, wouldn't it?)

Yes totally second change will be required where we actually start using images from ecr rather then docker hub. This is only first part, where we move the workflows and make Docker builds upload to both places ecr and dockerhub.

atalman · 2024-07-08T23:06:35Z

.ci/docker/common/install_xpu.sh

@@ -1,26 +1,30 @@
 #!/bin/bash
 set -xe



hi @chuanqi129 could you please review these changes in this file.

Thanks @atalman LGTM

.ci/docker/common/install_rocm_magma_docker.sh

jithunnair-amd · 2024-07-09T21:13:07Z

.ci/docker/common/install_mkl.sh

@@ -0,0 +1,15 @@
+#!/bin/bash


@atalman Any plans to sync this up with the mkl version being installed via install_conda.sh? There is a version difference (2021.4.0 for install_conda.sh vs 2024.2.0 for this file), a dynamic (install_conda.sh) vs static library (this file) difference, as well as an install location difference (/opt/conda/envs/py_$ANACONDA_PYTHON_VERSION for install_conda.sh vs /opt/intel for this file) .

hi @jithunnair-amd yes totally we should try to move to 2024.2.0. conda install will not work since we are not using conda-forge. However we should install these packages from pip

.ci/docker/common/install_rocm_magma.sh

jithunnair-amd · 2024-07-10T19:46:39Z

.ci/docker/common/install_mkl.sh

 MKL_VERSION=2024.2.0

-mkdir -p /opt/intel/
+MKLROOT=${MKLROOT:-/opt/intel}


@atalman IIUC, this script could end up using a different value of $MKLROOT (if env var already exists) as compared to .ci/docker/common/install_rocm_magma.sh, which will get it as /opt/intel from the ENV directive here. If that happens, the ROCm magma build would fail. Either we should hardcode MKLROOT to /opt/intel here, or set ARG MKLROOT /opt/intel as in my PR, since ARGs are visible in different stages of a multi-stage Dockerfile, but ENVs are limited to the specific stage where they are specified?

Sounds good let me use '/opt/intel' here . I don't think you are calling this file from rocm builds as far as I know.

* Add build-conda-images.yml in pytorch/pytorch * Add build_docker.sh to run .github/workflows/build-conda-images.yml * Create build-conda-images.yml * Delete .github/build-conda-images.yml Add build-conda-images.yml in pytorch/pytorch (#128563) (#128700) * Small adjustment for docker build * build_docker * Add docker file * Add common Add build-conda-images.yml in pytorch/pytorch (#128563) (#128962) * Add jni.h path * test * test * fixpath Update script path moving conda builds from builder to pytorch (#129167) * Add jni.h path * test * test * fixpath * test * test * test * test * test * test * test * test * test * test * tets * test * test Cleanup build docker images (#129273) * Cleanup docker move * add docker file Add calculate-docker-image when WITH_PUSH is true Update build-conda-images.yml Update build-conda-images.yml Update build-conda-images.yml test Update build-conda-images.yml Update build-conda-images.yml test test test test Update build-conda-images.yml Update build-conda-images.yml Update build-conda-images.yml Update build-conda-images.yml Update build-conda-images.yml Update build-conda-images.yml Update build-conda-images.yml Update build-conda-images.yml Update build-conda-images.yml Update build-conda-images.yml Update build-conda-images.yml Update build_docker_conda.sh test test test test test test test test test test test test test test test test test keep_both_tag test_conda_builds test test test test test test test test test

lint test common files test test test test test test test test test test test test test libtorch libtorch sudo test rocm fix test lint use one file for install_rocm_magma xpu rocm libtorch test libtorch test Manywheel builds fix_manywheel

atalman · 2024-07-22T13:19:24Z

.github/workflows/build-libtorch-images.yml

+    runs-on: linux.4xlarge
+    steps:
+      - name: Checkout PyTorch
+        uses: pytorch/pytorch/.github/actions/checkout-pytorch@move_docker_conda


temp code. Will update to main before merging

Hmm, why do you need to specify branch here to begin with? Shouldn't one just be using scripts from the current branch?

malfet

LGTM, but few comments:

Mixing CI and CD scripts in the same common folder feels a bit confusing, at least worth mentioning in the revie
Please do not use pytorch/pytorch/.github/actions/...@branch pattern, but rather relative ./github/actions/...
Hardcoding /home/ec2-user/actions-runner/_work feels wrong, and this line is not present in the builder repo, so why this is needed
Last but not least, according to https://github.com/pytorch/pytorch/blob/main/SECURITY.md#release-pipelines-security all release related activities should be done on the ephemeral runners (to prevent reverse ssh attacks), but linux.12xlarge is non-ephemeral, isn't it?

.github/workflows/build-conda-images.yml

malfet · 2024-07-22T16:32:01Z

.github/workflows/build-libtorch-images.yml

+      GPU_ARCH_VERSION: ${{ matrix.cuda_version }}
+    steps:
+      - name: Checkout PyTorch
+        uses: pytorch/pytorch/.github/actions/checkout-pytorch@move_docker_conda


Why specify branch if it could be done from the same repo?

Suggested change

uses: pytorch/pytorch/.github/actions/checkout-pytorch@move_docker_conda

uses: ./.github/actions/checkout-pytorch

Looks like this is what we use everywhere:

git grep "checkout-pytorch" github/actions/teardown-win/action.yml: # retry this step several time similar to how checkout-pytorch GHA does .github/workflows/_android-build-test.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_android-build-test.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_android-full-build-test.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_android-full-build-test.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_bazel-build-test.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_bazel-build-test.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_binary-build-linux.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_binary-test-linux.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_binary-upload.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_buck-build-test.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_buck-build-test.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main .github/workflows/_docs.yml: uses: pytorch/pytorch/.github/actions/checkout-pytorch@main etc.....

.github/workflows/build-libtorch-images.yml

.github/workflows/build-manywheel-images.yml

atalman · 2024-07-25T14:34:22Z

@pytorchmergebot merge -f "docker builds and lint are green"

pytorchmergebot · 2024-07-25T14:36:02Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

Summary: install_cuda.sh has been moved from pytorch/builder to pytorch/pytorch: pytorch/builder#1942 pytorch/pytorch#129022 We need to make sure that sm_90a is included: ``` xargs -I '{}' bash -c 'echo {} && /usr/local/cuda-12.4/bin/nvprune -gencode arch=compute_80,code=sm_80 -gencode arch=compute_86,code=sm_86 -gencode arch=compute_90,code=sm_90 -gencode arch=compute_90a,code=sm_90a /usr/local/cuda-12.4/lib64/{} -o /usr/local/cuda-12.4/lib64/{}' ``` Pull Request resolved: #2397 Test Plan: https://github.com/pytorch/benchmark/actions/runs/10163701096 Reviewed By: atalman Differential Revision: D60454551 Pulled By: xuzhao9 fbshipit-source-id: 03322314b190e0fb95e7d7741152de65ab35ecd4

juliagmt-google requested review from a team and jeffdaily as code owners June 19, 2024 01:15

pytorch-bot bot added the release notes: releng release notes category label Jun 19, 2024

pytorchbot added the open source label Jun 19, 2024

zou3519 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jun 21, 2024

pytorch-bot bot added the topic: not user facing topic category label Jun 21, 2024

atalman force-pushed the move_docker_conda branch 3 times, most recently from 3557afe to b80d939 Compare July 4, 2024 21:13

atalman changed the title ~~Update script path in .github/workflows/build-conda-images.yml~~ Migrate conda docker builds to pytorch/pytorch Jul 4, 2024

atalman force-pushed the move_docker_conda branch from b80d939 to f202745 Compare July 4, 2024 21:50

atalman approved these changes Jul 5, 2024

View reviewed changes

seemethere approved these changes Jul 5, 2024

View reviewed changes

malfet requested changes Jul 6, 2024

View reviewed changes

malfet requested review from atalman and seemethere July 6, 2024 13:22

atalman changed the title ~~Migrate conda docker builds to pytorch/pytorch~~ Migrate conda, manywheel and libtorch docker builds to pytorch/pytorch Jul 8, 2024

atalman reviewed Jul 8, 2024

View reviewed changes

.ci/docker/common/install_rocm_magma_docker.sh Outdated Show resolved Hide resolved

chuanqi129 approved these changes Jul 9, 2024

View reviewed changes

jithunnair-amd reviewed Jul 9, 2024

View reviewed changes

atalman reviewed Jul 10, 2024

View reviewed changes

.ci/docker/common/install_rocm_magma.sh Show resolved Hide resolved

jithunnair-amd reviewed Jul 10, 2024

View reviewed changes

atalman mentioned this pull request Jul 15, 2024

Cleanup docker image build workflows and scripts pytorch/builder#1923

Merged

juliagmt-google and others added 2 commits July 20, 2024 06:19

remove temp changes

6264e93

lint test common files test test test test test test test test test test test test test libtorch libtorch sudo test rocm fix test lint use one file for install_rocm_magma xpu rocm libtorch test libtorch test Manywheel builds fix_manywheel

atalman force-pushed the move_docker_conda branch from 10aa073 to 6264e93 Compare July 20, 2024 13:20

atalman reviewed Jul 22, 2024

View reviewed changes

malfet approved these changes Jul 22, 2024

View reviewed changes

atalman added 7 commits July 22, 2024 12:14

test

dea8fbc

lint and working dir

f9cffb6

test

4305069

test

00ebc51

Use ephemeral runners, changes from builder

8ca4096

Add linux.9xlarge to actionlint

94af48d

use ephemeral runners

608e533

pytorchmergebot added the merging label Jul 25, 2024

pytorchmergebot added the Merged label Jul 25, 2024

pytorchmergebot closed this in d0e2ab6 Jul 25, 2024

pytorchmergebot removed the merging label Jul 25, 2024

atalman mentioned this pull request Jul 29, 2024

[BE] cleanup install_cuda.sh script pytorch/builder#1942

Merged

xuzhao9 mentioned this pull request Jul 30, 2024

Download pytorch ci script to install cuda pytorch/benchmark#2397

Closed

henrylhtsang mentioned this pull request Jul 31, 2024

[BE][typing] fix types in common pruning #132309

Closed

atalman mentioned this pull request Aug 1, 2024

Reland "[BE] cleanup install_cuda.sh script"" pytorch/builder#1949

Merged

github-actions bot deleted the move_docker_conda branch August 25, 2024 02:04

atalman mentioned this pull request Dec 9, 2024

[BE] Migration of Builder Repository to pytorch/pytorch and pytorch/test-infra repository pytorch/builder#2054

Open

14 tasks

	uses: pytorch/pytorch/.github/actions/checkout-pytorch@move_docker_conda
	uses: ./.github/actions/checkout-pytorch

Migrate conda, manywheel and libtorch docker builds to pytorch/pytorch #129022

Migrate conda, manywheel and libtorch docker builds to pytorch/pytorch #129022

Uh oh!

Conversation

juliagmt-google commented Jun 19, 2024 • edited by atalman Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Jun 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/129022

❌ 1 New Failure

Uh oh!

malfet left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atalman commented Jul 8, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jithunnair-amd Jul 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atalman Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

malfet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

atalman commented Jul 25, 2024

Uh oh!

pytorchmergebot commented Jul 25, 2024

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

juliagmt-google commented Jun 19, 2024 •

edited by atalman

Loading

pytorch-bot bot commented Jun 19, 2024 •

edited

Loading

malfet left a comment •

edited

Loading

jithunnair-amd Jul 9, 2024 •

edited

Loading

atalman Jul 22, 2024 •

edited

Loading