Skip to content

[Klaud Cold] Re-introduce dsv4-fp8-mi355x-sglang on rocm/sgl-dev DSv4 image#2220

Open
Oseltamivir wants to merge 1 commit into
mainfrom
klaud-cold/dsv4-fp8-mi355x-sglang-restore
Open

[Klaud Cold] Re-introduce dsv4-fp8-mi355x-sglang on rocm/sgl-dev DSv4 image#2220
Oseltamivir wants to merge 1 commit into
mainfrom
klaud-cold/dsv4-fp8-mi355x-sglang-restore

Conversation

@Oseltamivir

Copy link
Copy Markdown
Collaborator

Summary

Re-introduces the dsv4-fp8-mi355x-sglang benchmark (DeepSeek-V4-Pro-FP8 on MI355X via SGLang), removed in #1501, with an updated DSv4-specific image.

Why it was removed, and why this works

#1501 removed the recipe because bumping it off the custom rocm/sgl-dev build to a generic lmsysorg/sglang-rocm release broke server boot:

ValueError: Downcasting not allowed: target.dtype=torch.float8_e4m3fn, loaded_weight.dtype=torch.bfloat16

DSV4-Pro-FP8 keeps some bf16 layers (shared-expert / embedding); only the DSv4-specific rocm/sgl-dev custom builds tolerate this. This PR pins to rocm/sgl-dev:rocm720-mi35x-da28108-20260610-DSv4 (a -DSv4 custom build), not a plain release. A config comment records this so future bumps do not naively re-break it.

Changes

  • configs/amd-master.yaml: restore dsv4-fp8-mi355x-sglang block on the new image (tp8, 1k1k + 8k1k, conc 4–64).
  • benchmarks/single_node/fixed_seq_len/dsv4_fp8_mi355x_sglang.sh: restore launch script under the current fixed_seq_len/ layout + _sglang naming (the old flat dsv4_fp8_mi355x.sh no longer matches launcher routing). Serving flags/env carried over from the last-known-working version (config model_type patch, --attention-backend compressed, --disable-cuda-graph, DSv4 FP8 env vars).

Test plan

  • full-sweep-fail-fast sweep passes (server boots on the new image without the downcast error; GSM8K eval passes).

Note: serving flags/env were validated against the April deepseek-v4-mi35x build; the June da28108-...-DSv4 build may need on-cluster tuning, which I will drive from the sweep results.

… image

Re-adds the dsv4-fp8-mi355x-sglang recipe and its launch script, removed in #1501 when a generic sglang-rocm image bump broke the DSV4-Pro-FP8 downcast check. Pins to the DSv4-specific custom build rocm/sgl-dev:rocm720-mi35x-da28108-20260610-DSv4 and restores the script under the current fixed_seq_len/ layout as dsv4_fp8_mi355x_sglang.sh.
@Oseltamivir Oseltamivir force-pushed the klaud-cold/dsv4-fp8-mi355x-sglang-restore branch from 0000f29 to 31678ac Compare July 15, 2026 12:33
@github-actions

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@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.

LGTM — straightforward re-introduction of the dsv4-fp8-mi355x-sglang recipe on the DSv4-specific image, following the standard recipe-add pattern.

What was reviewed: the new launch script's server flags/env vars against sibling dsv4 sglang scripts, the config-patch heredoc's local-path handling, the DP-attention/max-running-request/mem-fraction-static choices flagged by the bug hunter (checked against other dsv4/qwen mi355x sglang recipes and ruled out as intentional tuning choices, not bugs), and the amd-master.yaml / perf-changelog.yaml additions for consistency with existing entries.

Extended reasoning...

Overview

This PR re-adds a previously-removed benchmark recipe (dsv4-fp8-mi355x-sglang) that was pulled in #1501 after a generic image bump broke server boot on the DSV4-Pro-FP8 checkpoint's mixed bf16/fp8 layers. It pins to a DSv4-specific rocm/sgl-dev custom image tag instead, adds a launch script under benchmarks/single_node/fixed_seq_len/, a corresponding amd-master.yaml block, and a perf-changelog entry. This is exactly the shape of change the repo's own add-model-hardware/nuke workflows produce and is a very common, well-trodden PR pattern in this codebase.

Security risks

None. This is benchmark-harness configuration and a shell launch script for an internal GPU sweep; there is no user-facing surface, auth, or credential handling. The Python config-patch heredoc calls hf_hub_download with a repo id sourced from an internal config value, not untrusted input.

Level of scrutiny

Low-to-moderate is appropriate. This is a single self-contained benchmark recipe addition (config + script + changelog) with no changes to shared launcher/eval infrastructure, CI logic, or any code path outside this one recipe. It follows the same structure as dozens of sibling recipes already in amd-master.yaml, and the image pin plus rationale comment directly address the specific regression that caused the prior removal.

Other factors

The bug-hunting system found no bugs, and the candidate issues it raised (hardcoded --max-running-request, DP-attention always on, missing --mem-fraction-static, unconditional hf_hub_download call) were all examined and refuted as intentional recipe choices rather than defects. The PR description itself flags that serving flags may need on-cluster tuning based on sweep results, which is the normal iteration loop for this kind of recipe PR and not a blocker for merging — the accompanying full-sweep-fail-fast label will validate server boot and eval on real hardware.

@github-actions

Copy link
Copy Markdown
Contributor

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant