Conversation
…0924 Move the TP2 conc-16 point onto HiCache and pin hicache-size 253 on the srt-slurm recipe.
|
Thanks for the contribution!
中文感谢你的贡献！
|
|
View unofficial run (performance): https://inferencex.semianalysis.com/inference?unofficialRun=36242907012 View unofficial run (accuracy): https://inferencex.semianalysis.com/evaluation?unofficialRun=36242907012 |
yichiche
changed the title
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because configs/amd-master.yaml is CODEOWNERS-restricted to specific AMD reviewers (not including this PR's author), a human look from one of those owners would still be worthwhile before merging.
What was reviewed:
- Image tag bump (v0.5.19-...-20260915 to v0.5.20-...-20260924) is consistent between agentic.yaml's model.container and amd-master.yaml's image field.
- The TP2/TP4 conc-list restructuring: each of the 16 concurrency points across TP4 [1,4,8,12,16] resident, TP2 [1,4,8,12] resident, and TP2 [16,20,24,28,32,36,40] hicache maps to exactly one recipe override block (matching CONC/KV_OFFLOADING), with override_tp2_c16 renamed into override_tp2_c16_hicache and override_tp2_c20 (GPU-resident) deleted outright.
- hicache-size: 253 was added consistently to all 7 TP2 HiCache override blocks alongside the pre-existing hicache-ratio.
- perf-changelog.yaml only appends a new entry at the tail; no prior bytes were altered.
- Could not independently verify the new image tag exists on the registry (no outbound network access available in this environment).
Extended reasoning...
This is a config-only change (two YAML recipe/master-config files plus an appended changelog entry) that bumps an SGLang ROCm image tag and restructures the TP2 HiCache concurrency sweep for the Qwen3.5 MI355X AgentX recipe; no code, auth, or injection-relevant surface is touched. I verified the image bump is consistent across both files, that the conc-list changes map 1:1 to the recipe's override blocks with no benchmarked point silently dropped, that hicache-size was added uniformly, and that the changelog was only appended to. The deciding factor for deferring rather than approving is that configs/amd-master.yaml is covered by CODEOWNERS (specific named AMD reviewers), and the PR author is not among them, so a human owner should still sign off.
This review covers commit 4835978, which is no longer the latest commit on this pull request; later commits are not covered by it.
This branch has not been deployed
1 participant
Summary
qwen3.5-fp4-mi355x-sglang-agentic-mtpfromlmsysorg/sglang-rocm:v0.5.19-rocm720-mi35x-20260915tolmsysorg/sglang-rocm:v0.5.20-rocm720-mi35x-20260926(Docker Hub tag HTTP 200, digestsha256:f71b2a13a23132b9efdd39df58d13f310a59c2d101ec72271457295ed48016a9).[1, 4, 8, 12, 16]. Move TP2 conc 16 onto HiCache and drop TP2 GPU-resident conc 20. TP2 resident is[1, 4, 8, 12]; TP2 HiCache is[16, 20, 24, 28, 32, 36, 40].hicache-size: 253(overrideshicache-ratio) on every TP2 HiCache point.Details
Same recipe grid as #3455, with the image moved from
v0.5.20-rocm720-mi35x-20260924tov0.5.20-rocm720-mi35x-20260926:configs/amd-master.yaml: image and the TP2 conc grids for the AgentX arm.benchmarks/single_node/srt-slurm-recipes/qwen3.5/sglang/mi355x-fp4-mtp/agentic.yaml:model.containerset to the same image; residentoverride_tp2_c16andoverride_tp2_c20replaced byoverride_tp2_c16_hicache;hicache-size: 253added to every HiCache override.perf-changelog.yaml: one appended entry.Each of the 16 matrix points resolves to exactly one recipe override (gpus, CONC, KV_OFFLOADING, and image), and no override is left unused.
AI model disclosure
Cursor Grok 4.6 prepared the image bump, HiCache size pin, changelog, and this PR text.