-
Notifications
You must be signed in to change notification settings - Fork 15
Added Dockerfile for CI images #195
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
base: dev
Are you sure you want to change the base?
Conversation
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.
Please address the comments.
docker/Dockerfile
Outdated
RUN apt update \ | ||
&& apt install -y nano wget ninja-build \ | ||
&& apt install -y python3 python3-pip git \ | ||
&& apt install -y sqlite3 libsqlite3-dev libfmt-dev libmsgpack-dev libsuitesparse-dev |
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.
What are those packages (sqlite3 and further) for?
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 recall aotriton need sqlite3 long time ago, not sure about now
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 have derived this dockerfile from this https://github.com/ROCm/DeepLearningModels/blob/main/docker/pyt_semianalysis_models.ubuntu.amd.Dockerfile,
Will remove these, if these are not required.
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.
@VeeraRajasekhar Those are needed for the semianalysis models. We don't need them for the CI images. We only need to keep those packages necessary for building TE and running CI tests.
docker/Dockerfile
Outdated
|
||
RUN python3 -m pip install --upgrade pip | ||
RUN pip install ninja cmake setuptools wheel | ||
RUN pip install uv tabulate |
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.
What are those for?
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.
This is derived from https://github.com/ROCm/DeepLearningModels/blob/main/docker/pyt_semianalysis_models.ubuntu.amd.Dockerfile, I have removed these, will test the resultant docker after my changes
docker/Dockerfile
Outdated
RUN apt update \ | ||
&& apt install -y nano wget ninja-build \ | ||
&& apt install -y python3 python3-pip git \ | ||
&& apt install -y sqlite3 libsqlite3-dev libfmt-dev libmsgpack-dev libsuitesparse-dev |
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 recall aotriton need sqlite3 long time ago, not sure about now
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.
Why conversations are marked as resolved w/o any actual action?
Some of them, I have resolved, some I have currently resolved in my local, just to keep track I will mark them resolved. |
RUN apt install -y libzstd-dev | ||
RUN apt install -y libibverbs-dev | ||
|
||
ENV LLVM_SYMBOLIZER_PATH=/opt/rocm/llvm/bin/llvm-symbolizer |
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.
Why this LLVM_SYMBOLIZER_PATH need special assignment? For Pytorch installation?
ENV LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/opt/rocm/lib/: | ||
|
||
# Install pytorch | ||
# ARG PYTORCH_COMMIT="f929e0d602a71aa393ca2e6097674b210bdf321c" |
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.
Emm, if you comment out this PYTORCH_COMMIT ARG, how can we get that later in line 31 and 32
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.
There is build_tools/wheel_utils/ directory where NV scripts and docker files are. And also there is ci directory where CI scripts are. Consider putting this docker file in one of those locations
Description
Added the dockerfile, which can be used to create the ci-artifactory images.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: