-
Notifications
You must be signed in to change notification settings - Fork 18
Updated Dockerfile.hf for multi-arch build for amd64 and s390x #49
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
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates Dockerfile.hf to support multi-architecture builds for amd64 and s390x by parameterizing the base images with buildx’s TARGETPLATFORM and refactoring the file into a concise multi-stage build workflow. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Hi @ruivieira, could you please review this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m-misiura , can you please review this PR |
detectors/Dockerfile.hf
Outdated
PYTORCH_BUILD_VERSION=${TORCH_VERSION} PYTORCH_BUILD_NUMBER=1 pip wheel . --wheel-dir /torchwheels/ && \ | ||
pip install /torchwheels/*.whl; \ | ||
else \ | ||
pip install --no-cache-dir torch; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we pin torch version for consistency across architectures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this TORCH_VERSION (2.7.1), it is one of the latest and stable version for s390x architecture. As for amd64, I think it is showing torch>=2.0 in tox.ini file and for amd64 it may download the latest one
detectors/Dockerfile.hf
Outdated
pip install --no-cache-dir torch; \ | ||
fi | ||
|
||
|
||
FROM base as builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in our case, is there a significant advanatage of having a multi-stage build?
e.g. PyTorch must be present in the final image anyway (needed at runtime), so we can't eliminate it in a build stage
Main bulk is PyTorch itself; s390x build tools are relatively small compared to PyTorch there should be no large intermediate build artifacts to exclude?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the Dockerfile was already in mulit-stage build before this PR, and we have handled torch build for s390x case in the stage where torch is installed for amd64. We can also create a separate PR if we want to handle complete Dockerfile in a single stage build
@ruivieira @m-misiura Can you please review the PR |
thanks for your contribution @satyamg1620 -- could you please rebase the PR and then we'll merge it :) |
Sure @m-misiura |
c487a78
to
65ca4e6
Compare
@m-misiura Can you please review the PR |
Updated Dockerfile.hf for multi-arch build for amd64 and s390x.
Build logs:
amd64-hf-logs.txt
s390x-hf-logs.txt
Summary by Sourcery
Build: