-
Notifications
You must be signed in to change notification settings - Fork 501
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
build: update ascend dockerfile #2421
build: update ascend dockerfile #2421
Conversation
15277e5
to
c0dd202
Compare
c0dd202
to
0afd76c
Compare
docker/Dockerfile_aarch64_ascend
Outdated
git checkout ${LMDEPLOY_TAG_OR_COMMIT} && \ | ||
WORKDIR /opt/lmdeploy | ||
|
||
COPY autotest /opt/lmdeploy/autotest |
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 understand that src
has nothing to do with ascend inference.
But user might modify code and would like to commit to github.
So please keep it as a whole package
COPY . /opt/lmdeploy
WORKDIR /opt/lmdeploy
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.
COPY . /opt/lmdeploy
could bring all ascend *.run
files into docker images (about 3.8GB extra size for CANN 8.0.RC3.alpha001), I could not find a good way to exclude the ascend files.
Actually, there is an experimental syntax for Dockerfile that excludes certain patterns link
COPY --excludes *.run . /opt/lmdeploy
It's experimental and I have not tested yet, and may have extra requirements for docker version
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 understand that
src
has nothing to do with ascend inference. But user might modify code and would like to commit to github. So please keep it as a whole packageCOPY . /opt/lmdeploy WORKDIR /opt/lmdeploy
I think it's ok if extra 3.8GB size for ascend image is acceptable.
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.
@lvhan028 using a new multi-stage build avoid copy run file into final image, please review
Motivation
Using dlinfer instead of deeplink.framework in Dockerfile build for device=
ascend
Modification
Update Dockerfile_aarch64_ascend, using
pip3 install dlinfer-ascend==0.1.0
to supportascend
,torch_npu
is used internally.BC-breaking (Optional)
Does the modification introduce changes that break the backward-compatibility of the downstream repositories?
If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.
Use cases (Optional)
If this PR introduces a new feature, it is better to list some use cases here, and update the documentation.
Checklist