[Hardware][AMD][Perf][Bugfix] Update ROCr and clr in base image - #53712
Conversation
| ARG MORI_REPO="https://github.com/ROCm/mori.git" | ||
| ARG ROCPROFILER_SDK_REPO="https://github.com/ROCm/rocm-systems.git" | ||
| ARG ROCPROFILER_SDK_COMMIT="2b22ab0195cc1461cd9abf3b969e9dd7c10af350" | ||
| ARG ROCM_RUNTIME_REPO="https://github.com/mawong-amd/rocm-systems.git" |
There was a problem hiding this comment.
Use a rocm repo not the personal ones will be better?
Or update the base image of rocm/dev-ubuntu-22.04:7.2.3-complete to rocm/dev-ubuntu-22.04:7.14.0-full
There was a problem hiding this comment.
It's not so simple to do a full migration to ROCm 7.14: that also brings in newer hipBLASLt, RCCL, etc, all of which needs to be verified for performance and numerical accuracy. See #49925 which has been a work in progress for a while.
The ROCm 7.2.4 runtimes are essentially the ROCm 7.2.3 runtimes with some bolt-on fixes/improvements and are drop-in safe. On top of ROCm 7.2.4, the runtime here also carries an incipient fix with large performance implications for some stream dependencies. With that in mind, it makes sense to bring it in as soon as possible, after appropriate testing on vLLM workloads.
…ng edge signals Dockerfile.rocm_base builds the HSA runtime and clr from mawong-amd/rocm-systems, pinned at c7e40e16bca10311af72aee09cfa0cd1726f70a7, and installs the resulting shared libraries over the ones the ROCm 7.2.3 base image ships. The stage is defined before base and installs into base, not into final. Every FROM base stage compiles and links against the runtime it will run on. Installing only into final would leave torch, triton, flash-attention, aiter and mori built against the image's headers but running against these libraries. Signed-off-by: Matthew Wong <Matthew.Wong2@amd.com>
435f340 to
12be5eb
Compare
AndreasKaratzas
left a comment
There was a problem hiding this comment.
This is actually an emergency patch. We have agreed to a proper follow up PR. So we will merge this for now and soon we'll redirect to the official rocm repo.
…-project#53712) Signed-off-by: Matthew Wong <Matthew.Wong2@amd.com> Signed-off-by: khushali9 <khushali.desai9@gmail.com>
…-project#53712) Signed-off-by: Matthew Wong <Matthew.Wong2@amd.com>
…-project#53712) Signed-off-by: Matthew Wong <Matthew.Wong2@amd.com> Signed-off-by: mikeshawcode <michaelwshaw2@gmail.com>
…-project#53712) Signed-off-by: Matthew Wong <Matthew.Wong2@amd.com> Signed-off-by: mikeshawcode <michaelwshaw2@gmail.com>
…-project#53712) Signed-off-by: Matthew Wong <Matthew.Wong2@amd.com>
|
Data point from a different workload, in case it helps calibrate expectations: we overlaid exactly the two libraries this PR builds (
(*coincided with a 235 MB/s model download on the host.) So no measurable change for this TP4 / sparse-MLA / MTP configuration — the DeepSeek-V4 TP8 gains presumably come from the stream-dependency pattern that our path does not have. Correctness unchanged (needle tests to 900k tokens, perplexity). Not a complaint, just a "your mileage may vary" for TP4 setups. |
…-project#53712) Signed-off-by: Matthew Wong <Matthew.Wong2@amd.com>
Purpose
This PR updates ROCR and CLR in the ROCm 7.2.3 vLLM base image to backport in some bugfixes for graph replay (which was segfaulting under replay of complex graphs under some workloads) and for kernel dispatch latency under some stream dependencies.
In particular, the latter fix improves decode performance measurably on a few workloads (particularly
--async-schedulingand ModelRunner V2):Test Plan
GSM8K was measured on a number of models and configs on both gfx950 and gfx942 and in line with numbers before the runtime change. Also, full AMD CI will be triggered on this PR.
Test Result
There should not be any regressions in AMD CI caused by this PR.
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.