Add Dockerfile for ARM64 architecture support and update README instructions#827
Add Dockerfile for ARM64 architecture support and update README instructions#827z4y4ts wants to merge 2 commits intohuggingface:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for building Docker images on ARM64 architecture (Apple Silicon) to enable local development on Apple M-series machines. The PR addresses issue #611 where building the Docker image on Apple M4 chips failed due to Intel-specific dependencies not being available for ARM64.
Changes:
- Added new
Dockerfile-arm64specifically designed for ARM64 architecture without Intel MKL dependencies - Updated README instructions to reference the new ARM64-specific Dockerfile
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Dockerfile-arm64 | New Dockerfile for ARM64 builds, removing Intel MKL support and using ARM64-compatible sccache and protobuf binaries |
| README.md | Updated Docker build command to reference Dockerfile-arm64 for ARM64 platforms |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ``` | ||
| docker build . -f Dockerfile --platform=linux/arm64 | ||
| docker build . -f Dockerfile-arm64 --platform=linux/arm64 |
There was a problem hiding this comment.
The CI/CD workflow files (.github/workflows/build.yaml and .github/workflows/test.yaml) currently trigger on changes to "Dockerfile" but not "Dockerfile-arm64". This means that changes to the new ARM64 Dockerfile won't trigger automated builds or tests. Consider updating the workflow paths to include "Dockerfile-arm64" to ensure proper CI/CD coverage for ARM64 builds.
|
|
||
| ``` | ||
| docker build . -f Dockerfile --platform=linux/arm64 | ||
| docker build . -f Dockerfile-arm64 --platform=linux/arm64 |
There was a problem hiding this comment.
The matrix.json file used for automated builds doesn't include an entry for the ARM64 architecture. While this PR enables manual ARM64 builds for local development, consider adding an ARM64 entry to matrix.json (similar to the existing "cpu" entry but using "Dockerfile-arm64") to enable automated ARM64 Docker image builds and publishing in the CI/CD pipeline.
|
|
||
| ``` | ||
| docker build . -f Dockerfile --platform=linux/arm64 | ||
| docker build . -f Dockerfile-arm64 --platform=linux/arm64 |
There was a problem hiding this comment.
Consider adding documentation about the ARM64 Dockerfile in the docs/source/en/custom_container.md file, similar to how CPU and CUDA builds are documented. This would help users understand when and how to use Dockerfile-arm64, especially for Apple Silicon development environments.
|
Hey @z4y4ts thanks for opening the PR! Given that Metal won't work over Docker, which is really the point on having a separate Dockerfile for it? What's really the benefit here, other than making it a bit lighter due to the lack of Intel MKL libs? I'm just not sure about having a separate Dockerfile for it, but happy to discuss further in case it's a real need here, thanks! |
|
Hey @alvarobartt ! The key use case is to be able to do a local development on an Apple Silicone machines. We're deploying the service to a linux cluster. Local development is done with using https://tilt.dev/ The key need is to have dev setup as close to the production as possible. So the slower performance is an acceptable trade-off as the load on dev machine is expected to be low. As long as it provides the same features/models. I've added a separate Dockerfile because I see I'm not the only one with the same need. So it seems to be beneficial for the community. Also, the solution described in the README does not actually work. So it's not super obvious how to make it run and takes quite a few extra steps. If we agree this change is actually valuable, I'd like to also add it to the matrix.json to simplify macos setup even further. |
alvarobartt
left a comment
There was a problem hiding this comment.
Fair enough @z4y4ts thanks for the detailed information! I'll be happy to merge this PR and update the matrix.json to include it and build + push the arm64- image too (which I'll do in a follow up PR, as otherwise the CI won't run) 🤗
|
@alvarobartt thanks for approving! Asking because there's no merge button for me. |
What does this PR do?
This will make it easier to use the TEI on Apple Silicone machines.
The new
Dockerfile-arm64is based on theDockerfile. The differences are:Fixes #611
Before submitting
instasnapshots?Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.