Skip to content

Conversation

@VeeraRajasekhar
Copy link
Contributor

Description

Added the dockerfile, which can be used to create the ci-artifactory images.

Fixes # (issue)

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • Added a new file docker/Dockerfile

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Collaborator

@wenchenvincent wenchenvincent left a 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.

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
Copy link
Collaborator

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?

Copy link
Collaborator

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

Copy link
Contributor Author

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.

Copy link
Collaborator

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.


RUN python3 -m pip install --upgrade pip
RUN pip install ninja cmake setuptools wheel
RUN pip install uv tabulate
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are those for?

Copy link
Contributor Author

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

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
Copy link
Collaborator

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

Copy link
Collaborator

@ipanfilo ipanfilo left a 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?

@VeeraRajasekhar
Copy link
Contributor Author

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
Copy link
Collaborator

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"
Copy link
Collaborator

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

Copy link
Collaborator

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

@wenchenvincent
Copy link
Collaborator

@VeeraRajasekhar Is this PR still needed?

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.

5 participants