Conversation
…handles resolve on AMD GPUs A HIP IPC handle carries the EXPORTING process's device ordinal, and hipIpcOpenMemHandle resolves that ordinal against the IMPORTING process's device list. CUDA instead honours the importer's current device. Colocate weight sync therefore breaks on ROCm whenever a rollout engine is launched with a mask narrower than the trainer's: the trainer sees the whole node and exports at ordinal N, while the engine enumerates a single device where ordinal N is out of range. Weight sync succeeds on GPU 0 and fails on every other GPU with hipErrorInvalidValue. _compute_server_args built the engine subprocess's _visible_devices from the engine's own GPUs alone. Route it through _engine_visible_devices(), which lists the engine's own GPUs first and the rest of the node's afterwards. The engine still lands on its intended devices because its own are first; only the mask's LENGTH matters for ordinal resolution, not any agreement between the two numberings. Gated on torch.version.hip: on CUDA the narrow mask is correct and widening it would be a needless behaviour change. Verified on 8x MI308X (gfx942), ROCm 7.14, torch 2.12.0+rocm7.14, vLLM v0.29.0, colocate vLLM rollout + Megatron trainer. Single-variable A/B with 4 actor GPUs and --rollout-num-gpus-per-engine 1, so the engines are launched narrower than the trainer: without this commit three of four engines fail the first weight sync with a 500 from /update_weights and CUDA error: invalid argument in rebuild_cuda_tensor -> UntypedStorage._new_shared_cuda; with it the run exits 0 and all four engines sync. A framework-free two-process probe isolates the same ordinal resolution in ~10s. Co-authored-by: Claude Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Treemann <rongzhang.zheng@amd.com>
docker/Dockerfile.rocm targets ROCm 7.0.2 / gfx950 and builds the whole stack onto ubuntu:22.04 across nine stages. On a host that already has a prebuilt ROCm 7.14 image most of that work is done, so this variant starts from rocm/primus:v26.4 - which ships torch, triton, flash-attention, TransformerEngine and aiter - and adds only vLLM, Megatron-LM and vime. Single stage, no build context: every source is a pinned upstream git coordinate with its commit asserted, and nothing is COPYed in. VIME_REF accepts a branch or a full 40-hex sha; a sha is asserted, a branch has its resolved sha printed. Two properties are deliberate. The base image's ROCm torch is never replaced. A pip constraints file is generated from the installed versions right after the vLLM build, and every later pip install runs under it. vime's requirements.txt asks for vllm-router, transformers and ray[default] unpinned; without the constraints a resolver can satisfy one of them with a PyPI torch or vllm wheel - those are CUDA-only - and the breakage surfaces much later as a runtime failure on the first GPU op. A pip install --dry-run guard aborts the build if the resolver still plans to do it. A verification layer fails the build rather than the first training run: ROCm torch untouched, VLLM_TAG actually installed, vime/Megatron/Ray/ torch_memory_saver importing, and the vLLM API window vime needs present at both ends. That last check is why the VLLM_TAG pin is a checked claim and not a comment: vime needs NCCLTrainerInitInfo from vllm.distributed.weight_transfer.nccl_engine, which v0.27.1 predates, and it needs whichever cli_args location its current revision imports - the check reads that from vime's own source so it stays meaningful as main advances. Additive: docker/Dockerfile.rocm is untouched. Co-authored-by: Claude Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Treemann <rongzhang.zheng@amd.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds a Dockerfile for AMD ROCm 7.14 and updates the GPU visibility mask generation in vllm_engine.py to prevent HIP IPC handle resolution failures on ROCm. Feedback on the implementation highlights a potential issue where device indices could exceed the node's GPU count if the base index is offset, and suggests using a modulo operation to wrap the indices safely.
Signed-off-by: Rongzhang Zheng <rongzhang.zheng@amd.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
1. Colocate weight sync fails on every GPU but 0 (ROCm)
On ROCm a HIP IPC handle carries the exporter's device ordinal, and
hipIpcOpenMemHandleresolves it in the importer's enumeration; CUDA honours the importer's current device instead.
In colocate mode the trainer exports and the rollout engine imports, so a trainer rank that sees the
whole node exports at ordinal N while an engine launched with
HIP_VISIBLE_DEVICES=<its own gpu>enumerates one device where ordinal N does not exist. The two agree only when N is 0 — hence weight
sync works on GPU 0 and fails elsewhere with
hipErrorInvalidValue.Not a vLLM defect: the engine's IPC consumer is correct, the mask it was launched with is too
narrow. Only the mask's length matters, so listing the engine's own GPUs first keeps it on its
intended devices while putting the trainer's ordinal back in range.
Gated on
torch.version.hip, so on CUDA the helper returns the byte-identical string the currentcode produces and nothing changes. The mask only differs when the engine is narrower than the node:
2. A ROCm 7.14 Dockerfile
docker/Dockerfile.rocmtargets ROCm 7.0.2 / gfx950 and builds the stack ontoubuntu:22.04acrossnine stages.
docker/Dockerfile.rocm7.14instead starts from a prebuilt ROCm 7.14 image(
rocm/primus:v26.4, which already ships torch, triton, flash-attention, TransformerEngine andaiter) and adds only vLLM, Megatron-LM and vime. Single stage, no build context — every source is a
pinned git coordinate with its commit asserted, nothing is
COPYed in. Additive;docker/Dockerfile.rocmis untouched.Two guards are worth calling out:
installed versions after the vLLM build and every later
pip installruns under it. vime'srequirements.txtleavesvllm-router,transformersandray[default]unpinned, and aresolver satisfying one of them with a PyPI
torch/vllmwheel — those are CUDA-only — breaksthe image in a way that only surfaces on the first GPU op. A
--dry-runcheck aborts the build ifthe resolver still plans to do so.
VLLM_TAGactually installed, all imports resolving, and the vLLM API window vime needs presentat both ends.
Verification
8× MI308X (gfx942), ROCm 7.14, torch
2.12.0+rocm7.14, vLLMv0.29.0built from source,Megatron-LM
1dcf0dafa+docker/amd_patch/latest/, colocate vLLM + Megatron, Qwen2.5-0.5B.End-to-end A/B, single variable. Same vime revision, vLLM, script and topology; only the fix
differs. The run uses 4 actor GPUs with
--rollout-num-gpus-per-engine 1, so each engine islaunched with a mask narrower than the trainer's. It runs one rollout step, far enough to reach the
first weight sync.
POST /update_weights→ 500,CUDA error: invalid argumentinrebuild_cuda_tensor→UntypedStorage._new_shared_cuda, on 3 of 4 engines (GPU 0's is fine — its ordinal is already 0)Reproduced twice per arm, from separately built images.
Mechanism, without vime. A two-process probe (one 4 KiB tensor, torch only, ~10 s) isolates the
ordinal resolution. The exporter sees all 8 GPUs and allocates on GPU 3. An importer with
mask=3then fails with
invalid device ordinal, while an importer withmask=3,0,1,2,4,5,6,7succeeds andreturns an exact checksum, so the mapping landed on the intended device. With
--target 0both armspass, which is the expected asymmetry.
Build.
docker/Dockerfile.rocm7.14builds from this branch's head (sha assertion passes,17 layers, ~3 min warm / ~10 min cold) and its verification layer reports:
That same image ran the A/B above, so both commits are verified together. The
cli_argsassertionreads which module this vime revision imports rather than hardcoding a path, so it stays
meaningful as
mainadvances.pre-commitpasses every hook on this branch's changes.pytest tests/(in the image, excludingtests/ci):19 failed, 611 passed, 13 skipped, 57 errors. None involve this PR. The two files covering the changed path pass —tests/utils/test_vllm_engine.py(65 passed, exercises_compute_server_argsand_visible_devices) andtests/utils/test_vllm_config.py(17 passed) — and every file that reporteda failure also passes on its own (
test_vllm_arguments.py28,test_train_data_utils.py10,test_qwen3_5_vl_native.py8;tests/utils/in random order 152). The whole-suite result ispre-existing import-order pollution:
ValueError: megatron.core.__spec__ is Nonefromfind_specafter another module imported it, plus pytest failing on filenames containing dots
(
test_qwen3_0.6B_parallel_check.py→No module named 'test_qwen3_0').Scope
torch.version.hipgate and the expression being unchanged, not on a CUDA run.VLLM_TAG=v0.29.0is a compatibility claim about a moving target, since vime's mainline follows vLLM nightly: vime needsNCCLTrainerInitInfo(absent before v0.28.0) andvllm.entrypoints.launchers.cli_args(the Sep-2026 migration).v0.29.0has both, and the verification layer asserts that at build time instead of leaving it to a comment.docker/Dockerfile.rocm'sVLLM_TAG=6e448d0ea(v0.27.1) now satisfies neither end, so the existing ROCm Dockerfile cannot build a working vime against currentmain. Not addressed here; happy to open an issue.Prior art
Same principle as ms-swift#9627, which reached it for cross-process RCCL peer resolution: on ROCm, never shrink the visibility mask, only reorder it.
AI assisted contribution
Per
CONTRIBUTING.md, this work was developed with AI assistance. The analysis, the helper, the Dockerfile, the probes and this description were produced with an AI agent driving the debug loop on the hardware above. The submitter has reviewed every changed line, built the images and run both A/B arms, and is responsible for the content — including the Scope section. Both commits carry:Checklist
pre-commitpassespytest— the tests covering this change pass; all failures are pre-existing (see above)