Add aarch64 torchvision build #4445
Conversation
|
@atalman is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
facebook-github-bot
added
the
CLA Signed
linux aarch64 Modifying for aarch64 Adding aarch64 builds adding aarch64 builds test test test Fix typo test1 test test1 test fix typo more aarch64 changes test Skip aarch64 errors for now Display conda information Add conda to path Test Arm64 builds test More conda changes temp test1 testc test test fix sed test11 setrepo Test test111 test44 moreaarch64 moreaarch64 test test111 test111 test33 test test sleep30min test Enable debugging test test remove arch continue.. test Aarch64 vision worklfow
atalman
force-pushed
the
aarch64_changes
branch
from
10cf813 to
f2fe3ab
Compare
| required: false | ||
| type: string | ||
| default: x64 | ||
| setup-miniconda: |
There was a problem hiding this comment.
IIUC this is always going to be false for aarch64 and true otherwise. In that case, should we just make this an env var and set it accordingly instead of exposing as an arg to the caller.
There was a problem hiding this comment.
I think having input parameter is better, since its more flexible and more clear to the user , given that we already expose setup-miniconda input parameter in setup binary builds action: https://github.com/pytorch/test-infra/blob/main/.github/actions/setup-binary-builds/action.yml#L21 , I want to avoid if possible defining one more if statement like this :
${{ inputs.architecture }} == 'aarch64'
| @@ -97,16 +107,34 @@ jobs: | |||
| repository: ${{ inputs.test-infra-repository }} | |||
| ref: ${{ inputs.test-infra-ref }} | |||
| path: test-infra | |||
| - uses: actions/checkout@v3 | |||
There was a problem hiding this comment.
IIUC per comment below, the setup script will occur in aarch64 docker and we wouldn't need to checkout builder anymore right?
There was a problem hiding this comment.
@osalpekar Correct, this won't be needed once we use aarch64 docker. Also its probably best to move aarch64 build scripts from builder to test-infra repo. Hence this will not be required anyways
| @@ -134,6 +162,9 @@ jobs: | |||
| run: | | |||
| set -euxo pipefail | |||
| source "${BUILD_ENV_FILE}" | |||
| if [[ ${{ inputs.architecture }} == 'ARM64' ]]; then | |||
| ${CONDA_RUN} conda install -y libpng jpeg | |||
There was a problem hiding this comment.
Should we add these in https://github.com/pytorch/vision/blob/main/packaging/pre_build_script.sh (vision pre-script) instead?
There was a problem hiding this comment.
Sounds good, let me do this in subsequent PR in torchvision
| @@ -361,6 +369,11 @@ def generate_wheels_matrix( | |||
| # Define default compute architectures | |||
| arches = ["cpu"] | |||
|
|
|||
| if os == "linux-aarch64": | |||
There was a problem hiding this comment.
So should with_cuda, with_rocm, etc. be set to false automatically if os = linux-aarch64?
There was a problem hiding this comment.
Hm I don't think its really needed, it will complicate logic further, since we don't actually set with_cuda or with_rocm anywhere in this script, we treat these as our input only variables.
There was a problem hiding this comment.
They're enabled by default though right? Feel free to structure this however you see fit, as long as the correct matrix is generated (maybe we should have a unit-test for this in a follow-up).
| @@ -55,6 +55,16 @@ on: | |||
| description: "The key created when saving a cache and the key used to search for a cache." | |||
| default: "" | |||
| type: string | |||
| architecture: | |||
| description: Architecure to build for x64 for default Linux, or ARM64 for Linux aarch64 builds | |||
There was a problem hiding this comment.
s/Architecure/Architecture/
Also, I propose to use x86_64 rather than x64, to match results of uname -m call on respective hosts
| description: Architecure to build for x64 for default Linux, or ARM64 for Linux aarch64 builds | |
| description: Architecture to build for x86_64 for default Linux, or aarch64 for Linux aarch64 builds |
There was a problem hiding this comment.
Sounds good @malfet let me do this!
.github/workflows/test_build_wheels_linux_aarch64_without_cuda.yml
Outdated
Show resolved
Hide resolved
Follow up after #4445 to build torchaudio
This is first PR to automate aarch64 domains build. Starting with torchvision.
Following this script :
https://github.com/pytorch/builder/blob/main/aarch64_linux/build_aarch64_wheel.py
cc @osalpekar @malfet @xncqr