.github: Add GitHub Actions workflow to build wheels by seemethere · Pull Request #50633 · pytorch/pytorch

seemethere · 2021-01-15T23:51:20Z

Adds an initial draft for a workflow that'll build our release binaries for Linux wheels

Signed-off-by: Eli Uriegas eliuriegas@fb.com

seemethere · 2021-01-19T20:30:22Z

+            "gpu_arch_type": gpu_arch_type,
+            "container_image": CONTAINER_IMAGES[arch]
+        })
+    return json.dumps({"include": matrix})


It's important that this is include and not includes

samestep · 2021-01-22T23:30:19Z

+      - id: set-matrix
+        run: |
+          # outputting for debugging purposes
+          python .github/scripts/generate_binary_build_matrix.py


Is there a reason to run the script twice rather than running once and echoing the output twice (the second time with ::set-output)?

I like outputting the matrix in the first run for debugging purposes so we can see what the actual json blob looks like. perhaps it’d be better to just store that json blob as an artifact for later debugging purposes?

I guess I just meant, why run the script twice when the output is the same? Why not instead do something like this?

MATRIX=$(python .github/scripts/generate_binary_build_matrix.py) echo "${MATRIX}" # outputting for debugging purposes echo "::set-output name=matrix::${MATRIX}"

Oh you could potentially do that, but running the script is at a fairly low cost

samestep · 2021-01-22T23:31:49Z

+        uses: actions/checkout@v2
+      - id: set-matrix
+        run: |
+          # outputting for debugging purposes


apparently omitting the name here causes GHA to use the first line of the shell script as the step name

samestep · 2021-01-22T23:32:13Z

+          MATRIX=$(python .github/scripts/generate_binary_build_matrix.py)
+          echo "::set-output name=matrix::${MATRIX}"


Is there a limit to the size of a string that can be passed this way as an output to the next job?

No limit afaik

samestep · 2021-01-25T17:45:04Z

+CUDA_ARCHES = [
+    "10.1",
+    "10.2",
+    "11.0"
+]
+
+ROCM_ARCHES = [
+    "3.10",
+    "4.0"
+]
+
+FULL_ARCHES = [
+    "cpu",
+    *CUDA_ARCHES,
+    *ROCM_ARCHES
+]
+
+CONTAINER_IMAGES = {
+    **{
+        # TODO: Re-do manylinux CUDA image tagging scheme to be similar to
+        #       ROCM so we don't have to do this replacement
+        gpu_arch: f"pytorch/manylinux-cuda{gpu_arch.replace('.', '')}"
+        for gpu_arch in CUDA_ARCHES
+    },
+    **{
+        gpu_arch: f"pytorch/manylinux-rocm:{gpu_arch}"
+        for gpu_arch in ROCM_ARCHES
+    },
+    "cpu": "pytorch/manylinux-cpu"
+}


I guess this is assuming that no cuda version shares its version number with a rocm version? is there a reason we don't just put "cuda" or "rocm" directly into the arch name?

It's because I don't want to do a transformation of the value from here to the build scripts, so basically it's to just keep the number as transparent as possible from this point to the build point.

Although a better solution would be to not pass a variable at all and just have it all depend on the container image where the arch version would be automatically baked in as a number into the container image.

This approached is outlined in this PR: #45091

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

codecov · 2021-01-26T06:05:41Z

Codecov Report

Merging #50633 (5a3be0f) into master (5adbace) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #50633      +/-   ##
==========================================
- Coverage   80.91%   80.91%   -0.01%     
==========================================
  Files        1926     1926              
  Lines      210291   210291              
==========================================
- Hits       170152   170149       -3     
- Misses      40139    40142       +3

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

seemethere · 2021-01-26T20:50:34Z

This is now ready for review, the workflow won't actually run (except on workflow_dispatch), but there'll be follow up PR to disable the wheel builds in the nightly CircleCI run and enable the cron trigger for this workflow (with uploads)

facebook-github-bot

@seemethere has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

facebook-github-bot

@seemethere has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

samestep · 2021-01-27T16:25:44Z

+
+jobs:
+  generate-build-matrix:
+    if: ${{ github.repository_owner == 'pytorch' }}


does this just mean "if not on a fork"?

Basically, this was added in response to #49934

samestep · 2021-01-27T16:29:29Z

+    container:
+      image: python:3.9


could do this, doesn't matter too much though:

Suggested change

container:

image: python:3.9

container: python:3.9

samestep · 2021-01-27T16:29:48Z

+    container:
+      image: ${{ matrix.container_image }}


same idea as above:

Suggested change

container:

image: ${{ matrix.container_image }}

container: ${{ matrix.container_image }}

although I'm not sure if this has the potential break things e.g. if matrix.container_image is itself a map rather than just a string

samestep · 2021-01-27T16:46:48Z

+    parser.add_argument(
+        "--no-build-suffix",
+        type=strtobool,
+        help="Whether or not to add a build suffix typically (+cpu)",


I found these helpstrings a bit hard to understand (similar suggestions for the other two below); maybe something like this?

Suggested change

help="Whether or not to add a build suffix typically (+cpu)",

help="Don't add a build suffix (e.g. '+cpu')",

samestep · 2021-01-27T16:47:32Z

+    parser.add_argument(
+        "--gpu-arch-type",
+        type=str,
+        help="GPU arch you are building for, typically (cpu, cuda, rocm)",


Suggested change

help="GPU arch you are building for, typically (cpu, cuda, rocm)",

help="GPU arch you are building for (e.g. cpu, cuda, rocm)",

samestep · 2021-01-27T16:48:23Z

+    parser.add_argument(
+        "--gpu-arch-version",
+        type=str,
+        help="GPU arch version, typically (10.2, 4.0), leave blank for CPU",


Suggested change

help="GPU arch version, typically (10.2, 4.0), leave blank for CPU",

help="GPU arch version (e.g. 10.2 or 4.0), leave blank for CPU",

facebook-github-bot · 2021-01-27T19:56:09Z

@seemethere merged this pull request in 1b7a4f9.

Summary: Based on #50633 and #51243. Things left to do: - [x] modify `.github/scripts/generate_binary_build_matrix.py` further - [x] add option for not iterating over Python version - [x] add `LIBTORCH_VARIANT` - [x] add option for cxx11 - [x] fix the artifact uploading - [x] remove `pull_request` hook before merging Pull Request resolved: #53292 Test Plan: [CI](https://github.com/pytorch/pytorch/actions/runs/665781075). Reviewed By: seemethere Differential Revision: D27189150 Pulled By: samestep fbshipit-source-id: ec91e1f0b75b8c93613d55801585ed975697be03

Summary: Signed-off-by: Eli Uriegas <eliuriegas@fb.com> Fixes #{issue number} Pull Request resolved: pytorch#50633 Reviewed By: samestep Differential Revision: D26083492 Pulled By: seemethere fbshipit-source-id: c133671b9cf5074539133ee79fca5c680793a85d

Summary: Based on pytorch#50633 and pytorch#51243. Things left to do: - [x] modify `.github/scripts/generate_binary_build_matrix.py` further - [x] add option for not iterating over Python version - [x] add `LIBTORCH_VARIANT` - [x] add option for cxx11 - [x] fix the artifact uploading - [x] remove `pull_request` hook before merging Pull Request resolved: pytorch#53292 Test Plan: [CI](https://github.com/pytorch/pytorch/actions/runs/665781075). Reviewed By: seemethere Differential Revision: D27189150 Pulled By: samestep fbshipit-source-id: ec91e1f0b75b8c93613d55801585ed975697be03

facebook-github-bot added the cla signed label Jan 15, 2021

seemethere force-pushed the seemethere/actions_wheels branch 16 times, most recently from 4d00b09 to fe294bb Compare January 19, 2021 20:20

seemethere mentioned this pull request Jan 19, 2021

manywheel: Upgrade git for use with GHA pytorch/builder#622

Merged

seemethere commented Jan 19, 2021

View reviewed changes

seemethere force-pushed the seemethere/actions_wheels branch from 0200170 to e9bcbc1 Compare January 22, 2021 00:02

samestep reviewed Jan 22, 2021

View reviewed changes

Comment thread .github/workflows/linux_wheel.yml

samestep reviewed Jan 22, 2021

View reviewed changes

samestep reviewed Jan 25, 2021

View reviewed changes

seemethere and others added 5 commits January 25, 2021 18:18

WIP actions workflow to build wheels

ddb3fae

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

Change remote path for artifact to recursive glob

0bf6e75

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

Add auto-generating matrix

3f5b3a4

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

add env variable to skip tests

07d8d35

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

arch -> arch_version

fd446bc

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

add version generation

e42efe4

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

seemethere force-pushed the seemethere/actions_wheels branch from f73d373 to e42efe4 Compare January 26, 2021 02:18

seemethere added 3 commits January 25, 2021 18:23

remove spaces, checkout github.head_ref

dd6cdd5

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

use script in the pytorch directory

3c13dca

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

add working-directory

62db957

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

fix version generation, re-order matrix

5a3be0f

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

seemethere changed the title ~~WIP actions workflow to build wheels~~ .github: Add GitHub Actions workflow to build wheels Jan 26, 2021

seemethere added the module: ci Related to continuous integration label Jan 26, 2021

seemethere requested a review from a team January 26, 2021 20:48

seemethere marked this pull request as ready for review January 26, 2021 20:48

facebook-github-bot reviewed Jan 26, 2021

View reviewed changes

some cleanup before undrafting

1e134f4

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

seemethere force-pushed the seemethere/actions_wheels branch from ce37fec to 1e134f4 Compare January 26, 2021 20:52

facebook-github-bot reviewed Jan 26, 2021

View reviewed changes

samestep approved these changes Jan 27, 2021

View reviewed changes

facebook-github-bot closed this in 1b7a4f9 Jan 27, 2021

facebook-github-bot added the Merged label Jan 27, 2021

samestep mentioned this pull request Mar 4, 2021

.github: Add workflow to build libtorch #53292

Closed

6 tasks

github-actions Bot deleted the seemethere/actions_wheels branch February 10, 2024 01:54

		MATRIX=$(python .github/scripts/generate_binary_build_matrix.py)
		echo "::set-output name=matrix::${MATRIX}"

	container:
	image: ${{ matrix.container_image }}
	container: ${{ matrix.container_image }}

	help="Whether or not to add a build suffix typically (+cpu)",
	help="Don't add a build suffix (e.g. '+cpu')",

	help="GPU arch you are building for, typically (cpu, cuda, rocm)",
	help="GPU arch you are building for (e.g. cpu, cuda, rocm)",

	help="GPU arch version, typically (10.2, 4.0), leave blank for CPU",
	help="GPU arch version (e.g. 10.2 or 4.0), leave blank for CPU",

Conversation

seemethere commented Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jan 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

seemethere commented Jan 26, 2021

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jan 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

seemethere commented Jan 15, 2021 •

edited

Loading

codecov Bot commented Jan 26, 2021 •

edited

Loading