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=36328743525 View unofficial run (accuracy): https://inferencex.semianalysis.com/evaluation?unofficialRun=36328743525 |
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.
…ge-20260924 # Conflicts: # perf-changelog.yaml
|
Sorry, over the weekend, there was 2 major refactors to clean up the technical debt accumalated over the past 11 months of moving at the speed of light. We don't see any major refactors in the forthseeable future besides cleaning up AMD multinode AgentX pile of bash. As much, due to the refactors, u would need to ask your agent to rebase from remote main@latest. Thank you in advance for ur understanding |
Summary
qwen3.5-fp4-mi355x-sglang-agentic-mtpfromlmsysorg/sglang-rocm:v0.5.19-rocm720-mi35x-20260915tolmsysorg/sglang-rocm:v0.5.20-rocm720-mi35x-20260927(Docker Hub tag HTTP 200, digestsha256:a2b277dff0be18113d4eb013b607aa281b6020c3fe3e240770eebe9935b2015f).[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 set to
v0.5.20-rocm720-mi35x-20260927: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.