Repository navigation
[CI][ROCm] Add opt-in TheRock builds for AMD CI - #56351
Conversation
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
|
/amd-ci run nightly |
|
✅ Triggered Buildkite AMD CI #12855 for commit |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
|
/ci run |
|
✅ Triggered Buildkite CI #88220 for commit |
| pytorch_arg="$(extract_arg_default PYTORCH_BRANCH)" | ||
| pytorch_vision_arg="$(extract_arg_default PYTORCH_VISION_BRANCH)" | ||
| pytorch_audio_arg="$(extract_arg_default PYTORCH_AUDIO_BRANCH)" | ||
| triton_arg="${triton_arg:-$(extract_arg_default TRITON_VERSION)}" |
There was a problem hiding this comment.
Where does this come from? Could it end up being empty?
There was a problem hiding this comment.
It comes from ARG TRITON_VERSION in the selected base Dockerfile. Dockerfile.rock_base currently pins it to 3.8.0+git4cff872c.rocm10.0.0. The script first looks for TRITON_BRANCH, then falls back to TRITON_VERSION. Neither current recipe leaves the result empty.
|
So, we could use this to trigger a nightly with the rock without the need to pull the usual shenanigans, i.e. no need for doing git stuff and forced pushing, yadda, yadda? Is that right @AndreasKaratzas? |
| rocm_version="$(rocm_version_from_base_image "${base_image_arg}")" | ||
| rocm_version="$(extract_arg_default ROCM_SDK_VERSION)" | ||
| rocm_version="${rocm_version:-$(rocm_version_from_base_image "${base_image_arg}")}" | ||
| triton_arg="$(extract_arg_default TRITON_BRANCH)" |
There was a problem hiding this comment.
Hmm, we have both of these (TRITON_BRANCH and TRITON_VERSION)? Does it matter if it ends up empty? Maybe we always need to have it in the dockerfile?
There was a problem hiding this comment.
They belong to different recipes: regular ROCm uses TRITON_BRANCH for its source build, while Rock uses TRITON_VERSION for its wheel installation. Each Dockerfile already declares its corresponding pin. Here, an empty extracted value would leave the dependency metadata blank; it isn’t passed back into the build or used to select the cached image. Keeping the actual dependency pinned in each Dockerfile is still necessary.
Yes, that’s the intent. On a commit containing this change, we can launch the normal AMD pipeline with VLLM_USE_ROCK=1, RUN_ALL=1, and NIGHTLY=1 to run its full configured suite against TheRock without editing Dockerfiles or force-pushing. /amd-ci run nightly currently sets only the coverage flags, so Rock still needs to be selected in the Buildkite environment. |
| VLLM_CI_REQUIRE_WORKSPACE_MOUNT \ | ||
| VLLM_TEST_COMMANDS \ | ||
| VLLM_CI_BRANCH \ | ||
| VLLM_USE_ROCK \ |
There was a problem hiding this comment.
Do we need VLLM_USE_ROCK, or can we make the behavior that we just set CI_ROCM_DOCKERFILE_BASE and CI_ROCM_DOCKERFILE to the ROCk dockerfiles to override Dockerfile.rocm and Dockerfile.rocm_base?
Basically, I am wondering if we can boil this PR down to just two additional env vars (CI_ROCM_DOCKERFILE_BASE and CI_ROCM_DOCKERFILE) to override the Dockerfiles. This would be nice in case we want to support multiple dockerfiles/ROCm versions in the future (e.g. then we could also use this PR directly when we want to start testing Dockerfile.rocm_base_gfx1250 and Dockerfile.rocm_gfx1250 in CI).
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
|
/ci run |
|
✅ Triggered Buildkite CI #88370 for commit |
There was a problem hiding this comment.
LGTM! This would be useful in running CI on candidate images and such moving forward (like the gfx1250 images, or new ROCm previews).
I'm going to kick off an AMD CI build with the following env vars to make sure it works, will post the results here
CI_ROCM_DOCKERFILE_BASE=docker/Dockerfile.rock_base
CI_ROCM_DOCKERFILE=docker/Dockerfile.rock
NIGHTLY=1
Edit: Seems to have run successfully here https://buildkite.com/vllm/amd-ci/builds/12883
|
/ci retry |
|
✅ Triggered Buildkite CI #88424 for commit |
|
This pull request has merge conflicts that must be resolved before it can be |
Resolve the Rock Dockerfile overlap using main's identical smoke-test stages. Preserve upstream Rock profiler library fixes alongside the PR's custom AMD CI Dockerfile selection, cache isolation, and runtime handoffs. Validation: pre-commit checks on all PR files, seven existing ROCm CI metadata/smoke tests, and shell configuration behavior checks. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
|
/ci run |
|
✅ Triggered Buildkite CI #89140 for commit |
|
/ci retry |
|
✅ Triggered Buildkite CI #89185 for commit |
Opt the AMD image steps into the TheRock Dockerfiles from vllm-project#56351 so the MI355 Elastic EP job runs on ROCm SDK 10.0.0 (HIP, RCCL, PyTorch) and we can see whether the HIP IPC failure is fixed in the newer ROCm userspace. Signed-off-by: aarushjain29 <aarushi.jain2@amd.com>
4 participants
CI_ROCM_DOCKERFILE_BASEandCI_ROCM_DOCKERFILEto select alternative Dockerfiles in the normal AMD Buildkite pipeline.NIGHTLY=1enabling the full configured nightly suite.For a Rock nightly, set these Buildkite build environment variables:
The regular ROCm Dockerfiles remain the defaults. This extends the existing AMD CI build path in this PR and lets other compatible Dockerfile pairs use the same maintained jobs.