fix(docker): build native binaries for every arch and un-pin the base image - #1
Open
maple52046 wants to merge 1 commit into
Open
fix(docker): build native binaries for every arch and un-pin the base image#1maple52046 wants to merge 1 commit into
maple52046 wants to merge 1 commit into
Conversation
… image
The image was effectively tied to one GPU architecture and to one specific torch
base. Three of those are correctness problems independent of any particular base:
- hip-stream passed no --offload-arch at all, leaving the target to hipcc's
detection -- and there is no GPU to detect during `docker build`.
- p2pBandwidthLatencyTest was deliberately built for the first arch only, so on a
host running any other arch in PYTORCH_ROCM_ARCH the binary simply does not run.
Its Makefile interpolates GPU_TARGETS into a single --offload-arch and an
unquoted ';' value is additionally split by the shell into a second command,
which surfaces as "ld.lld: error: undefined symbol: main". Passing the first arch
through GPU_TARGETS and the rest as extra --offload-arch flags via the Makefile's
CPPFLAGS passthrough produces a real fat binary with no Makefile or source edit.
- ENV PYTHONPATH="/opt/aiter:${PYTHONPATH}" leaves a trailing colon when the base
does not set PYTHONPATH. An empty entry means "current directory", so any
directory containing e.g. ./cmake shadows the installed module.
All three native binaries now cover every arch in PYTORCH_ROCM_ARCH, so one image
runs on gfx942 and gfx950 and nothing is compiled at run time.
The base image becomes an ARG, defaulting to the ROCm 7.2.4 / Python 3.13 /
PyTorch 2.12.0 combination this was verified against, and the two base-specific
steps are made self-checking so the file still works on other bases:
- the amdgpu-install-internal removal now runs only when that package is actually
installed (`apt remove` on a missing package exits non-zero and fails the build);
- cmake is reinstalled only when the base image's cmake is broken.
The hipBLASLt client .debs move to a pkgs/ subdirectory of the build context so
`COPY . /opt/benchtestkit` no longer drags them into the image.
Co-authored-by: Cursor <cursoragent@cursor.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.
Why
Built as-is, the image is tied to a single GPU architecture and to one specific torch base. Three of those are correctness problems that are independent of which base you use.
The three generic fixes
hip-streamwas built with no offload arch at all.vendor/babelstream-hip/HIP.makenever passes--offload-arch, so the target is left to hipcc's detection — and there is no GPU to detect duringdocker build. It does appendEXTRA_FLAGS, so the archs now go in as one repeated flag each.p2pBandwidthLatencyTestwas deliberately limited to the first arch. On a host running any other arch fromPYTORCH_ROCM_ARCHthe binary just does not run. The previous comment attributed this to the Makefile interpolatingGPU_TARGETSinto a single--offload-arch, which is true, but the actual failure with a;-separated value is that the shell splits the command in two:Everything after
;becomes a second command, so hipcc gets one arch and no source file — hence the confusingundefined symbol: main. The Makefile appendsCPPFLAGSverbatim, so passing the first arch throughGPU_TARGETSand the remaining ones as extra--offload-archflags yields a genuine fat binary with no Makefile or source change.ENV PYTHONPATH="/opt/aiter:${PYTHONPATH}"leaves a trailing colon when the base image does not setPYTHONPATH. An emptyPYTHONPATHentry means "current directory", so any directory containing e.g../cmakeshadows the installed module. Now only appended when there is something to append:/opt/aiteron a base without it,/opt/aiter:<inherited>on a base with it (both verified).Net effect: all three native binaries cover every arch in
PYTORCH_ROCM_ARCH, so one image runs on gfx942 and gfx950 and nothing is compiled at run time.Base image becomes an ARG
BASE_IMAGEnow defaults to the combination this was verified against: ROCm 7.2.4 / Python 3.13 / PyTorch 2.12.0 (rocm/ali-private:ubuntu22.04_rocm7.2.4_cp313_torch2.12.0_20260726). Labels andROCM_VERSIONwere updated to match that default.The two base-specific steps are made self-checking rather than deleted, so the file still works on the previous base:
apt remove -y amdgpu-install-internalnow runs only when that package is actually installed. It is legitimate for bases that carry the internal repo, butapt removeon a missing package exits non-zero and fails the build on bases that do not.cmakepackage under one interpreter while/usr/local/bin/cmakeruns another, sofrom cmake import cmakeresolves to the/opt/rocm/lib/cmakenamespace directory and raisescannot import name 'cmake' from 'cmake'. TransferBench and rccl-tests both need cmake.hipBLASLt client packages
The GEMM benchmarks need
hipblaslt-bench, and the public radeon repo does not publishhipblaslt-benchmarks— it only ever existed in the offline bundle. The.debsare still bind-mounted from the build context, but now from apkgs/subdirectory so thatCOPY . /opt/benchtestkitdoes not drag them into the image. Defaults point at the 1.2.2 client packages built from the matchingrocm-7.2.4tag; both areARGs.vLLM
Unchanged — still intentionally absent, per this repo's existing rationale.
paged_attentionuses the aiter backend; the commented-out opt-in block is untouched. This is now also stated in the image description label.Verification
Built and run on an MI355X (gfx950) host from the new default base.
benchtestkit check-envin the resulting image:roc-obj-lson each native binary reportsgfx942 gfx950— all three are fat binaries.Real workloads on the gfx950 host:
hipblaslt-bench -m 1024 -n 1024 -k 1024→ 55924 Gflops (f16)hip-stream→ Triad 3031719 MB/s (~3.0 TB/s)import aitersucceeds and JIT-compilesmodule_aiter_corein 9.0s;is_flydsl_available()isTrueThe framework stack is untouched by this image:
torch 2.12.0+rocm7.2.4.gitcf5ea6e,triton 3.7.1,numpy 2.5.1,flash_attn 2.8.4— identical to the base image. The only Python package added ispydantic 2.13.4(benchtestkit's own dependency) plus its transitive deps; nothing pulls or downgrades torch or numpy.Note for reviewers
BuildKit emits an
UndefinedVar: '$PYTHONPATH'lint warning on bases that do not set the variable. That is exactly the case${var:+...}exists to handle. Do not silence it by declaringARG PYTHONPATH— anARGshadows the inheritedENVand the append is lost (verified).