Conversation
|
View unofficial run (performance): https://inferencex.semianalysis.com/inference?unofficialRun=35071638854 View unofficial run (accuracy): https://inferencex.semianalysis.com/evaluation?unofficialRun=35071638854 |
There was a problem hiding this comment.
Looks good, straightforward config revert. Checked the model-checkpoint/golden-AL wiring end-to-end (models.yaml AgentX alias, server_sglang.sh's MODEL_NAME case statement, golden_al_distribution/dsv4_mtp.yaml) and the resulting thinking-on AL=2.49 for 3 draft tokens matches the plain checkpoint's committed curve; the perf-changelog entry appends cleanly at the tail with no historical bytes touched.
Extended reasoning...
Overview
The diff is two files: configs/amd-master.yaml reverts the model field of the dsv4-fp4-mi355x-sglang-disagg-agentic-hicache-mtp recipe from deepseek-ai/DeepSeek-V4-Pro-0813 back to deepseek-ai/DeepSeek-V4-Pro, and perf-changelog.yaml gets one new entry appended at the physical end of the file describing the change. No code, scripts, or other recipes are touched.
Verification performed
I traced the actual runtime effect of the model: field change rather than taking the PR description at face value:
runners/launch_mi355x-amds.shsetsMODEL_NAME=${MODEL##*/}(basename), so after the revertMODEL_NAMEbecomesDeepSeek-V4-Pro.benchmarks/multi_node/amd_utils/server_sglang.shresolvesmodel_key = f'{MODEL_NAME}-AgentX'againstmodels.yaml. There,DeepSeek-V4-Pro-0813-AgentX: *DeepSeek-V4-Pro-AgentXis a literal YAML alias ofDeepSeek-V4-Pro-AgentX— samebase_flags(including--enable-deepseek-v4-fp4-indexer), samemtp_flags(--speculative-algorithm EAGLE --speculative-eagle-topk 1). So reverting the model string does not change any served flags; the indexer flag was never checkpoint-specific, contradicting the one candidate issue that was raised and ruled out earlier in this run.- The synthetic-acceptance golden-AL selection in
server_sglang.shis acase "${MODEL_NAME}:${DECODE_MTP_SIZE}"match:DeepSeek-V4-Pro-0813:3 -> 3.01(the DSpark curve,golden_al_distribution/dsv4-pro-0813-dspark.yaml) vs the wildcard*DeepSeek-V4*:3 -> 2.49(the native-MTP curve,golden_al_distribution/dsv4_mtp.yaml, thinking_on/3 = 2.49). After the revert,MODEL_NAME=DeepSeek-V4-Profalls into the wildcard branch and correctly resolves to AL 2.49, exactly matching the new changelog entry's claim.
Security risks
None — config-only change to a benchmark recipe's model checkpoint string and an append-only changelog entry. No secrets, auth, or executable code paths.
Level of scrutiny
This is a narrowly-scoped, mechanical revert of a single field in a single recipe, and I independently reconciled the claimed behavior (golden AL 2.49) against the actual selection logic in three different files rather than trusting the PR description. The perf-changelog append is clean (verified via diff and line count: the new entry ends exactly at the file's last line, no historical entry altered), consistent with the repo's append-only invariant. The pull/XXX placeholder pr-link matches the exact template shown in CONTRIBUTING.md, so it is the documented convention rather than an error.
Other factors
The bug hunter reported zero findings, and the one candidate issue considered (the --enable-deepseek-v4-fp4-indexer flag pairing) is affirmatively falsified by the models.yaml alias structure. No CODEOWNERS restriction applies to config/changelog files in this repo layout, and there is no outstanding third-party objection in the conversation timeline.
This review covers commit b5ddd3c, which is no longer the latest commit on this pull request; later commits are not covered by it.
Correct the pr-link picked up from the cherry-picked #3170 entry now that this DeepSeek-V4-Pro-0813 DSpark change has its own PR. Signed-off-by: Theresa Shan <theresa.shan@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
The MI355X disaggregated recipe pairs Pro-0813 with the legacy EAGLE/MTP draft architecture, which logs missing required draft weights. Switch its model back to
deepseek-ai/DeepSeek-V4-Proso the existing EAGLE path uses the original checkpoint’s native MTP weights. The existing selector restores thinking-on golden AL 2.49 for three draft tokens; evaluations retain real acceptance.Also collect server logs from every Slurm node.
/run_logsis node-local, and the old launcher copied only node 0’s tree, leaving decode logs out of GitHub artifacts. After the server step and container cleanup, run one collection task per node into the shared directory used by the existing artifact packager. Preserve server failures, fail otherwise-successful jobs if collection fails, and restore runner ownership. This adapts the fix proposed in #3167.Explicitly pass the existing SGLang router port into evaluation. The shared library now requires
PORTbefore parsing--port; without this caller input, evaluation exits after successful server startup. The endpoint remains port 30000. Forward the workflow-ownedAIPERF_EXPERIMENTAL_FASTandREQUIRE_POWERvalues through the existing container environment allowlist; both are explicitly0for this sweep. This restores the intended client inputs without introducing defaults or changing validation.The serving image and seven-point sweep remain unchanged. A perf changelog entry and
full-sweep-enabledrequest fresh results.Validation:
git diff --checkpass; historical changelog bytes are preserved.Runtime checks confirmed complete native MTP loading on both roles, golden AL 2.49 in the throughput command, and both prefill/decode logs in an uploaded server artifact. GPU qualification is still in progress after fixing the missing client inputs. Confirm complete draft weight loading, real acceptance during evaluation, and both roles in uploaded server-log archives before treating the rerun as validated.
Follow-up to #2823.