Skip to content

fix(amd): restore native MTP checkpoint and collect all node logs - #3170

Closed
cquil11 wants to merge 6 commits into
mainfrom
codex/amd-dsv4-restore-native-mtp
Closed

cquil11 wants to merge 6 commits into
mainfrom
codex/amd-dsv4-restore-native-mtp

Conversation

@cquil11

@cquil11 cquil11 commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

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-Pro so 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_logs is 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 PORT before parsing --port; without this caller input, evaluation exits after successful server startup. The endpoint remains port 30000. Forward the workflow-owned AIPERF_EXPERIMENTAL_FAST and REQUIRE_POWER values through the existing container environment allowlist; both are explicitly 0 for 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-enabled request fresh results.

Validation:

  • All seven expected matrix points use the original checkpoint, MTP, and two nodes.
  • Cluster inspection confirms all 64 original checkpoint shards and the required native MTP tensor names are present.
  • Behavioral node-log staging tests: 2 passed, including separate prefill/decode trees and a missing-source failure.
  • Real eval-library reproduction confirms the port fix uses the existing router endpoint.
  • Bash syntax, Ruff lint/format, perf changelog/matrix validation, and git diff --check pass; 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.

@cquil11 cquil11 added full-sweep-enabled AMD agentx AgentX benchmarks, recipes, and infrastructure labels Sep 16, 2026
@cquil11 cquil11 changed the title fix(amd): restore original DeepSeek-V4-Pro for native MTP fix(amd): restore native MTP checkpoint and collect all node logs Sep 16, 2026
@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.sh sets MODEL_NAME=${MODEL##*/} (basename), so after the revert MODEL_NAME becomes DeepSeek-V4-Pro.
  • benchmarks/multi_node/amd_utils/server_sglang.sh resolves model_key = f'{MODEL_NAME}-AgentX' against models.yaml. There, DeepSeek-V4-Pro-0813-AgentX: *DeepSeek-V4-Pro-AgentX is a literal YAML alias of DeepSeek-V4-Pro-AgentX — same base_flags (including --enable-deepseek-v4-fp4-indexer), same mtp_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.sh is a case "${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-Pro falls 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.

ichbinblau added a commit that referenced this pull request Sep 16, 2026
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>
@cquil11

cquil11 commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as superseded per maintainer direction: use #3188 (Pro-0813 with DSPARK) as the serving baseline for the srt-slurm port in #2555. This does not claim successful E2E validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agentx AgentX benchmarks, recipes, and infrastructure AMD full-sweep-enabled

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant