[AMD] [AgentX] Add MiniMax-M3 MXFP8 EAGLE3 on MI325X - #2579
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
1 similar comment
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
cquil11
added
AMD
agentx
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| lmcache) | ||
| require_agentic_kv_offload_backend lmcache | ||
| LMCACHE_VERSION="0.5.3" | ||
| LMCACHE_ROCM_INDEX="https://github.com/LMCache/LMCache/releases/expanded_assets/v${LMCACHE_VERSION}-rocm" | ||
| agentic_pip_install --quiet --no-cache-dir --no-deps \ | ||
| "sortedcontainers==2.4.0" \ | ||
| "opentelemetry-exporter-prometheus==0.61b0" \ | ||
| "cupy-rocm-7-0==14.1.1" \ | ||
| "lmcache==${LMCACHE_VERSION}" --find-links "$LMCACHE_ROCM_INDEX" | ||
| python3 -c \ | ||
| "import cupy; import lmcache.integration.vllm.lmcache_mp_connector; import opentelemetry.exporter.prometheus" \ | ||
| >/dev/null | ||
|
|
||
| LMCACHE_N_SERVERS="$TP" | ||
| LMCACHE_L1_SHARD_GB=$((TOTAL_CPU_DRAM_GB / LMCACHE_N_SERVERS)) | ||
| if [ "$LMCACHE_L1_SHARD_GB" -lt 1 ]; then | ||
| echo "Error: LMCache DRAM budget is less than 1 GB per TP rank." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| LMCACHE_SERVER_URLS=() | ||
| LMCACHE_HTTP_PORTS=() | ||
| LMCACHE_LOGS=() | ||
| : > "$RESULT_DIR/lmcache_command.txt" | ||
| for shard in $(seq 0 $((LMCACHE_N_SERVERS - 1))); do | ||
| shard_port=$((5555 + shard)) | ||
| shard_http_port=$((8080 + shard)) | ||
| shard_log="${LMCACHE_LOG%.log}_${shard}.log" | ||
| LMCACHE_CMD=( |
There was a problem hiding this comment.
🟡 sweep:supported-transfer-mode lmcache_driven — this ~30-line LMCache-MP spawn block (pinned pip installs, import probe, per-shard lmcache server loop, wait_for_ready loop, kv-transfer-config JSON) in minimaxm3_fp8_mi325x_mtp.sh:68-96 is copied verbatim from minimaxm3_fp4_mi355x_mtp.sh. Factor it into a benchmark_lib.sh helper parameterized by TP/DRAM budget via env vars per AGENTS.md, so the /dev/shm guard drift (flagged separately) can't recur.
Extended reasoning...
What/where: benchmarks/single_node/agentic/minimaxm3_fp8_mi325x_mtp.sh (lines 68-96, the lmcache) case branch) reproduces the LMCache-MP server-spawn logic near-verbatim from minimaxm3_fp4_mi355x_mtp.sh: the same pinned pip install set (sortedcontainers==2.4.0, opentelemetry-exporter-prometheus==0.61b0, cupy-rocm-7-0==14.1.1, lmcache==0.5.3 from the same ROCm find-links index), the same python3 -c "import cupy; import lmcache..." probe, the same per-TP-rank for shard in $(seq 0 ...) loop spawning lmcache server on ports 5555+shard / 8080+shard with identical flags (--l1-init-size-gb 10 --l1-read-ttl-seconds 7200 --chunk-size 256 --max-workers 2 --eviction-policy LRU --supported-transfer-mode lmcache_driven), the same wait_for_ready health-check loop, and a byte-identical kv-transfer-config JSON payload for LMCacheMPConnector. A third variant of the same spawn logic exists in dsv4_fp4_mi355x_vllm_mtp.sh.
Why this is the AGENTS.md convention, not a nitpick about style: AGENTS.md states verbatim: "Shared benchmark Bash behavior belongs in benchmark_lib.sh, with parameters passed through environment variables." The repo already follows this for wait_for_ready, append_command, write_command, stop_background_process_tree, and require_agentic_kv_offload_backend — all live in benchmark_lib.sh and are sourced by every recipe script. The LMCache-MP spin-up block is exactly the kind of behavior that convention targets: it is identical business logic (spawn N lmcache servers sized to a DRAM budget, wait for them, assemble the connector config) that only varies by TP-derived variable names across call sites, yet it was pasted into a third file instead of being lifted into a helper like start_lmcache_mp_servers "$TP" "$TOTAL_CPU_DRAM_GB".
Proof this isn't hypothetical — it has already caused drift: Compare the two copies directly. minimaxm3_fp4_mi355x_mtp.sh's copy of this block includes a /dev/shm capacity guard before spawning the LMCache servers (added later, per the sibling PR discussion referenced in the companion bug_001 finding) — a check that this new minimaxm3_fp8_mi325x_mtp.sh copy does not have. Concretely:
- A future engineer fixes a resident-memory OOM in the fp4 script by adding a
/dev/shmsize check ahead of the spawn loop. - They test only that script, ship the fix, and move on — there is no single source location that would have forced them to touch the fp8 copy too.
- This fp8 script (and the dsv4 script) keep spawning
lmcache serverprocesses without that guard, so the same OOM class can resurface here even though it was "fixed" upstream. - The next maintainer, unaware a fix already exists, either rediscovers the bug from scratch or re-implements a slightly different guard in a fourth location.
Why nothing today prevents this: there is no benchmark_lib.sh entry point for LMCache-MP spin-up (wait_for_ready is shared, but the surrounding spawn/probe/config-assembly logic is not), so each new MTP+LMCache recipe script is authored by copy-pasting the nearest sibling rather than calling a shared function. bash -n and the PR's own validation steps (YAML parsing, matrix generation) cannot catch behavioral drift between copies — only a shared helper can.
Suggested fix: extract a benchmark_lib.sh function, e.g. start_lmcache_mp_servers <n_servers> <total_dram_gb> <log_prefix>, that performs the pip install, import probe, per-shard spawn loop, wait_for_ready loop, and returns/exports the assembled kv-transfer-config JSON (plus any capacity guard). All LMCache-MP recipe scripts — this one, minimaxm3_fp4_mi355x_mtp.sh, and dsv4_fp4_mi355x_vllm_mtp.sh — would then call the same helper with TP/DRAM-budget parameters passed as arguments or env vars, matching the pattern already used for require_agentic_kv_offload_backend and friends. This is a code-quality/DRY finding, not a functional bug in this PR, so it doesn't block merge.
| LMCACHE_N_SERVERS="$TP" | ||
| LMCACHE_L1_SHARD_GB=$((TOTAL_CPU_DRAM_GB / LMCACHE_N_SERVERS)) | ||
| if [ "$LMCACHE_L1_SHARD_GB" -lt 1 ]; then | ||
| echo "Error: LMCache DRAM budget is less than 1 GB per TP rank." >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🟡 Missing /dev/shm capacity check before sizing LMCache L1 shards: LMCACHE_L1_SHARD_GB in minimaxm3_fp8_mi325x_mtp.sh (lines 81-86) is derived straight from TOTAL_CPU_DRAM_GB/LMCACHE_N_SERVERS with only a >=1GB-per-shard sanity check. The sibling script this was copied from, minimaxm3_fp4_mi355x_mtp.sh (lines 104-111), additionally probes free /dev/shm via df -BG --output=avail /dev/shm, caps the request at 90% of that, and exits early with a clear error — this new script drops that guard, so an oversized DRAM budget will instead surface as an unclear failure deep in lmcache/tmpfs allocation and risks exhausting shared host memory for co-located jobs.
Extended reasoning...
What the bug is. minimaxm3_fp8_mi325x_mtp.sh computes the per-shard LMCache L1 size directly from the operator-supplied TOTAL_CPU_DRAM_GB env var and the TP-derived LMCACHE_N_SERVERS, then only checks that the resulting per-shard value is at least 1 GB:
LMCACHE_N_SERVERS="$TP"
LMCACHE_L1_SHARD_GB=$((TOTAL_CPU_DRAM_GB / LMCACHE_N_SERVERS))
if [ "$LMCACHE_L1_SHARD_GB" -lt 1 ]; then
echo "Error: LMCache DRAM budget is less than 1 GB per TP rank." >&2
exit 1
fiThere is no check that the aggregate requested size (LMCACHE_L1_SHARD_GB * LMCACHE_N_SERVERS, effectively TOTAL_CPU_DRAM_GB) actually fits in the host's /dev/shm.
Where this diverges from the pattern it was copied from. The sibling recipe benchmarks/single_node/agentic/minimaxm3_fp4_mi355x_mtp.sh (same EAGLE3/LMCache MP setup, same lmcache_driven transfer mode) has exactly the missing guard at lines 104-111, run before sizing the shards:
SHM_FREE_GB=$(df -BG --output=avail /dev/shm 2>/dev/null | tail -1 | tr -dc '0-9')
if [ -n "$SHM_FREE_GB" ] && [ "$SHM_FREE_GB" -gt 0 ]; then
SHM_CAP_GB=$((SHM_FREE_GB * 90 / 100))
if [ "$LMCACHE_L1_SIZE_GB" -gt "$SHM_CAP_GB" ]; then
echo "Error: LMCache L1 ${LMCACHE_L1_SIZE_GB} GB exceeds 90% of free /dev/shm (${SHM_CAP_GB} GB)." >&2
exit 1
fi
fiI verified this block still exists in the sibling script at those exact lines. This PR's new script reuses the same lmcache server ... --supported-transfer-mode lmcache_driven construction, per-shard loop, and wait_for_ready polling as the sibling, but the /dev/shm probe was dropped when the LMCache block was ported over.
Why this matters. LMCache's lmcache_driven L1 backing store for the MP connector lives in tmpfs (/dev/shm), which is a resource shared with the OS and any co-located processes on the runner — it is not the same pool as TOTAL_CPU_DRAM_GB (regular process/page-cache DRAM). Without the cap, a TOTAL_CPU_DRAM_GB value that's large relative to the runner's actual /dev/shm size will pass the trivial >= 1GB per shard check and proceed to spawn lmcache server processes that attempt to allocate more tmpfs than is physically backed. This recipe's own PR description flags a convention-derived ~599 GB DRAM budget (from dram-utilization: 0.20), which is a plausible trigger if the mi325x-amds runner's /dev/shm is sized smaller than that.
Concrete walk-through. Suppose the runner has /dev/shm sized at 400 GB (smaller than the ~599 GB budget) and TP=8:
LMCACHE_L1_SHARD_GB = 599 / 8 = 74GB/shard — passes the>= 1check trivially.- Eight
lmcache server ... --l1-size-gb 74 ...processes are launched in parallel, each attempting to reserve tmpfs backing. - Aggregate requested tmpfs (~592 GB) exceeds the 400 GB actually available; some shard(s) fail deep inside lmcache's tmpfs allocation, or the host itself experiences memory pressure from over-committing shared memory.
- Failure is only detected later via
wait_for_ready's health-check timeout / dead-PID check againstLMCACHE_LOGS, producing an opaque timeout instead of the sibling script's immediate, actionable"Error: LMCache L1 ... exceeds 90% of free /dev/shm ..."message. - Worse, if other jobs are co-located on the same host, the oversized tmpfs request can degrade or OOM those unrelated jobs rather than just this one.
How to fix. Port the sibling's /dev/shm probe verbatim into this script, before (or alongside) the existing per-shard-minimum check: query df -BG --output=avail /dev/shm, cap the requested aggregate at 90% of that, and exit with a clear error if the budget exceeds the cap.
Severity. This is a robustness/diagnostics regression relative to the sibling pattern, not a correctness bug in the benchmark data itself — when the guard would fire, the run fails either way, just with a much worse error message and added risk to co-located jobs. Given the recipe deliberately uses a conservative dram-utilization: 0.20, the guard is unlikely to trip in practice on the intended runner. Recommending nit.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31620921458 |
cquil11
added
non-canary-full-sweep-enabled
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31625179679 |
cquil11
removed
non-canary-full-sweep-enabled
024a4d4 to
7eeea89
Compare
cquil11
added
the
non-canary-full-sweep-enabled
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31641957752 |
1 participant
Summary
Mooncake is intentionally excluded because its AMD TCP store previously collapsed mid-replay in run 30517360090. Once the LMCache gate is functional, this PR will launch the complete broad resident fast matrix without fail-fast or premature pruning.
Validation
bash -ngit diff --check