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

Make array-record build on aarch64 #72

Closed
wants to merge 1 commit into from

Conversation

joker-eph
Copy link

@joker-eph joker-eph commented Aug 8, 2023

In the absence of an Aarch64 wheel (see #71), this helps building from source

  • upgrade highwayhash: the previous revision was 4y ago, there has been quite a few fixes since then.

  • python/BUILD does not seem properly handle by copybara, it refers multiple paths inside google3 apparently.

  • python/array_record_data_source.py does not include array_record but is using it. Maybe some other copybara problem?

One more problem left somehow is that running bazel test isn't hermetic: the python test will prefer to import array_record from the environment installation (system or pip) instead of the local repo.

@Marvin182
Copy link
Collaborator

I think this collides with changes we did yesterday to release a new wheel. See bf51831.

This should address the problems in python/BUILD and python/array_record_data_source.py. I will make a separate change to upgrade highwayhash.

This leaves some changes in the oss/build_whl.sh.
a) BAZEL_FLAGS+="@sigbuild-r2.9-${PYTHON}_config_cuda//crosstool:toolchain"
=> I'm not sure why we have this. I will check with the team.

b) BAZEL_FLAGS="--crosstool_top="
=> what is the issue with this?

c) manylinux2014_x86_64 => linux_aarch64
I'm still confused about the right way of releasing Python packages for various the platforms. According to PEP 600 we should have something like:
manylinux_${GLIBCMAJOR}${GLIBCMINOR}${ARCH} and either go with glibc 2.17 or 2.28 (those seem common and manylinux2014 is an alias for manylinux_2_17). So should we do:
manylinux_2_17_x86_64 and manylinux_2_17_aarch64 or go with the newer 2.28?

@joker-eph
Copy link
Author

Thanks for the update!
Glad to see some fixes in this direction, I'll test tomorrow and rebase this to clear out the diff :)

This leaves some changes in the oss/build_whl.sh.
a) BAZEL_FLAGS+="@sigbuild-r2.9-${PYTHON}_config_cuda//crosstool:toolchain"
=> I'm not sure why we have this. I will check with the team.
b) BAZEL_FLAGS="--crosstool_top="
=> what is the issue with this?

These are going together, bagel flag is [email protected]${PYTHON}_config_cuda//crosstool:toolchain
This instructs Bazel which toolchain to use. I don't know if this toolchain could also support Arm64, I am also not sure how to provide it (I suspect you may get it from some TensorFlow docker container? If so some instructions could be useful).

I'm building on an Arm64 machine within a docker container, and I am just fine using the toolchain available locally in the container.

One easy way out would be to check if BAZEL_FLAGS is set in the environment by the user and not setting the crosstool you're using if this is the case. I could invoke the build script with BAZEL_FLAGS= oss/build_whl.sh.

c) manylinux2014_x86_64 => linux_aarch64
I'm still confused about the right way of releasing Python packages for various the platforms. According to PEP 600 we should have something like:
manylinux_${GLIBCMAJOR}${GLIBCMINOR}${ARCH} and either go with glibc 2.17 or 2.28 (those seem common and manylinux2014 is an alias for manylinux_2_17). So should we do:
manylinux_2_17_x86_64 and manylinux_2_17_aarch64 or go with the newer 2.28?

To be honest, I don't know :)

In my case, I don't need to publish a wheels that is portable, I just want pip install . to work.

If it helps: I have no problem building and installing jaxlib (and Jax), it works the exact same way on X86 and Arm64 machines. Could this be an example to follow?

@cantonios
Copy link
Contributor

a/b) is to synchronize with the build config for TF (apparently TF 2.9 though). We would need to sync up with AWS (who are maintaining official TF arm releases) to figure out their build configuration and docker image for ARM. It might be linaro/tensorflow-arm64-build:2.14-multipython for the latest TF release, which is what we use for the nightly CI.

c) Again, we would need to sync up with ARM/AWS, but it looks like they are using manylinux_2_17_aarch64 [release links]

I think we can work around the local build failure issues by having variables for the crosstool and auditwheel platform (and skip those steps if not set).

copybara-service bot pushed a commit that referenced this pull request Aug 22, 2023
Related to #72 - allows building the pip package locally on ARM (or any other
platform).

- Added environment variables for the crosstool toolchain (`CROSSTOOL_TOP`) and
  the auditwheel platform (`AUDITWHEEL_PLATFORM`).
- Created a Docker file for ARM builds.
- Modified build scripts to allow building either locally (`build_whl.sh`),
  or with the docker container (`docker_runner.sh`).
- Updated TensorFlow dependency to 2.12.1.  ARM builds are only released
  starting at TF 2.12.

Closes #72.

PiperOrigin-RevId: 559191606
copybara-service bot pushed a commit that referenced this pull request Aug 22, 2023
Related to #72 - allows building the pip package locally on ARM (or any other
platform).

- Added environment variables for the crosstool toolchain (`CROSSTOOL_TOP`) and
  the auditwheel platform (`AUDITWHEEL_PLATFORM`).
- Created a Docker file for ARM builds.
- Modified build scripts to allow building either locally (`build_whl.sh`),
  or with the docker container (`docker_runner.sh`).
- Updated TensorFlow dependency to 2.12.1.  ARM builds are only released
  starting at TF 2.12.

Closes #72.

PiperOrigin-RevId: 559191606
copybara-service bot pushed a commit that referenced this pull request Aug 22, 2023
Related to #72 - allows building the pip package locally on ARM (or any other
platform).

- Added environment variables for the crosstool toolchain (`CROSSTOOL_TOP`) and
  the auditwheel platform (`AUDITWHEEL_PLATFORM`).
- Created a Docker file for ARM builds.
- Modified build scripts to allow building either locally (`build_whl.sh`),
  or with the docker container (`docker_runner.sh`).
- Updated TensorFlow dependency to 2.12.1.  ARM builds are only released
  starting at TF 2.12.

Closes #72.

PiperOrigin-RevId: 559191606
copybara-service bot pushed a commit that referenced this pull request Aug 23, 2023
Related to #72 - allows building the pip package locally on ARM (or any other
platform).

- Added environment variables for the crosstool toolchain (`CROSSTOOL_TOP`) and
  the auditwheel platform (`AUDITWHEEL_PLATFORM`).
- Created a Docker file for ARM builds.
- Modified build scripts to allow building either locally (`build_whl.sh`),
  or with the docker container (`docker_runner.sh`).
- Updated TensorFlow dependency to 2.12.1.  ARM builds are only released
  starting at TF 2.12.

Closes #72.

PiperOrigin-RevId: 559191606
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants