[ROCm] Add TheRock preview docker updates, Keep Python 3.12 and Ubuntu 22.04 - #49925
[ROCm] Add TheRock preview docker updates, Keep Python 3.12 and Ubuntu 22.04#49925rasmith wants to merge 71 commits into
Conversation
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
| export HIP_CLANG_PATH=/opt/sccache-wrappers \ | ||
| && sccache --show-stats; \ | ||
| fi \ | ||
| && PREBUILD_KERNELS=1 AITER_USE_SYSTEM_TRITON=1 GPU_ARCHS=${AITER_ROCM_ARCH} python3 setup.py bdist_wheel --dist-dir=dist \ |
There was a problem hiding this comment.
Should aiter also be pip installable?
There was a problem hiding this comment.
I'd recommend we push that to a follow-up ... we haven't throughly tested the AITER prebuilt wheels yet
There was a problem hiding this comment.
It seems to work. I am running the full CI right now. We will need to do some additional testing. @tjtanaa @Rohan138 You can get the images from the CI and do additional testing if you want: https://buildkite.com/vllm/amd-ci/builds/12262/list
| | grep -E '^(torch|triton|torchaudio|torchvision)==' > /etc/rocm-constraints.txt \ | ||
| && echo "ROCm constraints:" && cat /etc/rocm-constraints.txt \ | ||
| && test "$(wc -l < /etc/rocm-constraints.txt)" -eq 4 \ | ||
| && python3 -c "import torch; assert torch.version.hip, f'non-ROCm torch: {torch.__version__}'; print(f'torch={torch.__version__}')" \ |
There was a problem hiding this comment.
I assume this came from debugging the torch/triton compatibility issues, can we remove this comment+rocm_constraints+torch/triton assert and print now?
There was a problem hiding this comment.
I'm working on this part at the moment. I"ll keep the comments short.
There was a problem hiding this comment.
Simplification done, will post soon.
| && TRITON_VERSION=$(python3 -c "from importlib.metadata import version; print(version('triton'))") \ | ||
| && sed -i "s/^Requires-Dist: triton==.*/Requires-Dist: triton==${TRITON_VERSION}/" \ | ||
| ${SITE_PACKAGES}/torch-*.dist-info/METADATA \ | ||
| # Editing METADATA stales its RECORD hash; restamp so wheel verifiers pass. |
There was a problem hiding this comment.
Do we need any of this after the sed, or was the METADATA also edited for debug purposes?
There was a problem hiding this comment.
torch pins triton==3.7.1, so this prevents it from resolving it to a version of triton we don't want. It also prevents other resolvers from seeing the pin as unsatisfiable and then installing triton 2.13 with CUDA.
I still need to revisit this to see if a constraints file will work.
There was a problem hiding this comment.
I have a simplification, but the source builds don't add the METADATA and RECORD information that cause unpreventable reinstalls, so we do need to patch that in-place.
| RUN --mount=type=bind,from=debs,src=/app/debs,target=/install \ | ||
| pip install $(ls /install/*.whl | grep -v '/triton-') \ | ||
| && pip install --no-deps --force-reinstall /install/triton-*.whl \ | ||
| # Point torch's pin at the triton we built. Left unsatisfied, downstream |
There was a problem hiding this comment.
What's the downstream resolver here? Is it a subsequent pip install either in this file or in Dockerfile.rock?
There was a problem hiding this comment.
There's a lot of resolver issues with this Triton installation. Found this out by trying to get a Triton 3.6.0 and it repeatedly got swapped out for 3.7.1, I think at least at 5 different place, most (maybe all) in Dockerfile.rock. If you're not careful, you will lose the Triton you wanted. I had to use constraints files.
There was a problem hiding this comment.
OK, I have a simplification that I will be uploading shortly.
| && curl -sS https://bootstrap.pypa.io/get-pip.py | python${PYTHON_VERSION} \ | ||
| && python3 --version && python3 -m pip --version | ||
|
|
||
| RUN pip install -U packaging 'cmake<4' ninja wheel 'setuptools<80' pybind11 Cython |
There was a problem hiding this comment.
Can we copy over the pybind pin to 3.0.4 and the new Triton commit from https://github.com/vllm-project/vllm/pull/52819/changes? 52819 will be merged first, just waiting on CI
There was a problem hiding this comment.
@Rohan138 What do you mean by "copy over pybind pin to 3.0.4"?
| ARG COMMON_WORKDIR=/app | ||
| ARG BASE_IMAGE=rocm/vllm-dev:base | ||
| ARG CI_BASE_IMAGE=rocm/vllm-dev:ci_base | ||
| ARG ROCM_TRITON_KERNELS_COMMIT=0f380657dbf3ee86eb57558ff71df24f03b5d4e7 |
There was a problem hiding this comment.
Out of curiosity, why did we pin TRITON_KERNELS to this specific commit?
There was a problem hiding this comment.
hmm ... @AndreasKaratzas @micah-wil this won't hold up this PR, but I'm not sure if we should pin triton_kernels in our docker image for CI?
| ARG FA_REPO="https://github.com/Dao-AILab/flash-attention.git" | ||
| ARG AITER_BRANCH="v0.1.19" | ||
| ARG AITER_REPO="https://github.com/ROCm/aiter.git" | ||
| ARG MORI_BRANCH="f95cd5e0ca7b730f92f9ffdfb358484d6965d9f7" # TheRock compatible v1.1.0 |
There was a problem hiding this comment.
Do we have a MORI PR up for this change?
There was a problem hiding this comment.
I was working on one, but there's this PR: ROCm/mori#541. Also that change was already in MORI, but not MORI 1.1.0, maybe we could bump, but might be best to wait for the next version which should have support for The Rock.. However, the PR I linked is pretty much tip of tree.
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
| ARG TRITON_REPO="https://github.com/ROCm/triton.git" | ||
| ARG FA_BRANCH="0e60e394" | ||
| ARG FA_REPO="https://github.com/Dao-AILab/flash-attention.git" | ||
| ARG AITER_BRANCH="v0.1.19.post2" |
There was a problem hiding this comment.
Let's revert to AITER 0.1.19 to align with upstream, I'll bump both dockerfiles to 0.1.20 as part of #52826
Signed-off-by: Randall Smith <Randall.Smith@amd.com>
Purpose
This PR adds to using The Rock 7.14 with wheels provided by The Rock as a preview version while keeping Python 3.12 and Ubuntu 22.04 as-is.
Test Plan
Full CI runs.
Test Result
Current persistently failing groups that need to be addressed are:
Keeping track in this BK build:https://buildkite.com/vllm/amd-ci/builds/12222/list
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.