Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docker/rocm.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@

# Default base images
ARG BASE_IMAGE_942="rocm/sgl-dev:rocm7-vllm-20250904"
ARG BASE_IMAGE_942_ROCM720="rocm/pytorch:rocm7.2_ubuntu22.04_py3.10_pytorch_release_2.9.1"
ARG BASE_IMAGE_942_ROCM720="rocm/pytorch:rocm7.2.2_ubuntu22.04_py3.10_pytorch_release_2.9.1"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The upgrade to ROCm 7.2.2 in the base image may be ineffective if the TORCH_ROCM_FILE (defined at line 462 as torch-2.9.1+rocm7.2.0...) is not also updated. This wheel is force-installed at line 517, potentially overwriting the PyTorch installation in the new base image with one built against ROCm 7.2.0. If this wheel bundles its own ROCm libraries or was compiled against the buggy hipEventQuery implementation, the fix for the race condition described in the PR may not be realized in the final image. Additionally, if this specific wheel file is not present in the new base image's filesystem (as implied by the path in hack.py), the build will fail.

ARG BASE_IMAGE_950="rocm/sgl-dev:rocm7-vllm-20250904"
ARG BASE_IMAGE_950_ROCM720="rocm/pytorch:rocm7.2_ubuntu22.04_py3.10_pytorch_release_2.9.1"
ARG BASE_IMAGE_950_ROCM720="rocm/pytorch:rocm7.2.2_ubuntu22.04_py3.10_pytorch_release_2.9.1"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The variable names and build stages (e.g., BASE_IMAGE_950_ROCM720 and gfx950-rocm720) still use the 720 suffix, which is now inconsistent with the 7.2.2 version. More importantly, the pattern matching logic in several RUN blocks (lines 121, 149, 514) specifically checks for *rocm720*. If a user attempts to build with GPU_ARCH=gfx950-rocm722 to match the new version, these critical configuration and patching steps will be skipped. Consider updating the naming and pattern matching to be more generic (e.g., rocm72*) to avoid maintenance issues with future patch releases.


# This is necessary for scope purpose
ARG GPU_ARCH=gfx950
Expand Down
Loading