Skip to content

[DO NOT MERGE][WIP][NV] update- qwen3.5-fp8-b200-sglang-mtp #1513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hshrivastava-droid wants to merge 2 commits into from
Closed

[DO NOT MERGE][WIP][NV] update- qwen3.5-fp8-b200-sglang-mtp #1513

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e21ae18
qwen3.5-fp8-b200-sglang-mtp: drop --enable-symm-mem
hshrivastava-droid May 18, 2026
86c5a29
update PR number
hshrivastava-droid May 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion benchmarks/single_node/qwen3.5_fp8_b200_mtp.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ set -x
SGLANG_ENABLE_SPEC_V2=1 PYTHONNOUSERSITE=1 python3 -m sglang.launch_server --model-path=$MODEL --host=0.0.0.0 --port=$PORT \
--trust-remote-code \
--tensor-parallel-size=$TP --data-parallel-size=1 --expert-parallel-size=$EP_SIZE \
--enable-symm-mem \
--disable-radix-cache \
--quantization fp8 \
--kv-cache-dtype fp8_e4m3 \
Expand Down
6 changes: 6 additions & 0 deletions perf-changelog.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2841,3 +2841,9 @@
description:
- "Add Qwen-3.5-397B-A17B FP8 sglang recipes (off + MTP/EAGLE) for H100 on lmsysorg/sglang:v0.5.12-cu130"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1509

- config-keys:
- qwen3.5-fp8-b200-sglang-mtp
description:
- "Drop --enable-symm-mem from qwen3.5 FP8 B200 sglang MTP server launch"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX

Check warning on line 2849 in perf-changelog.yaml

View check run for this annotation

Claude / Claude Code Review

perf-changelog pr-link uses placeholder XXXX 

The new perf-changelog entry has `pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX` (line 2849) — the `XXXX` placeholder was never substituted. Every other entry in the file uses a real PR number (the entry just above uses `/pull/1509`); please replace `XXXX` with `1513` (this PR's number) so the link resolves and any tooling that parses `pr-link` for audit/navigation continues to work.

@claude claude Bot May 18, 2026

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.

🟡 The new perf-changelog entry has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX (line 2849) — the XXXX placeholder was never substituted. Every other entry in the file uses a real PR number (the entry just above uses /pull/1509); please replace XXXX with 1513 (this PR's number) so the link resolves and any tooling that parses pr-link for audit/navigation continues to work.

Extended reasoning...

What the bug is

The newly added perf-changelog.yaml entry sets:

- config-keys:
    - qwen3.5-fp8-b200-sglang-mtp
  description:
    - "Drop --enable-symm-mem from qwen3.5 FP8 B200 sglang MTP server launch"
  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX

The literal string XXXX is an unsubstituted template placeholder, not a real PR number. The PR description itself also contains /pull/XXXX in the same form, indicating the author simply forgot to substitute the number before opening the PR.

Why this is a real (if minor) issue

Every other entry in perf-changelog.yaml uses a real PR number — e.g. the immediately preceding entry (line 2843) uses /pull/1509, and earlier entries use /pull/1491, /pull/1422, etc. This is a consistent, file-wide convention. The XXXX link is a guaranteed 404 and breaks any tooling, audit script, or human navigation that follows pr-link to find the originating PR.

Step-by-step proof

  1. Open perf-changelog.yaml and scroll to the bottom — the diff adds lines 2844–2849.
  2. Line 2849 contains pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX.
  3. Visiting that URL returns a 404 because XXXX is not a valid issue/PR number.
  4. Compare to the entry just above (line 2843): pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1509 — this resolves correctly to PR [Klaud Cold] Add qwen3.5-fp8-h100-sglang (off + mtp) recipes #1509.
  5. The current PR's number is 1513 (per the PR metadata), so substituting XXXX → 1513 produces the correct link.

How to fix

Replace XXXX on line 2849 with 1513:

  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1513

Severity rationale

This is a documentation/metadata defect — it does not affect the recipe change itself (the --enable-symm-mem removal in qwen3.5_fp8_b200_mtp.sh is unaffected). Flagging as nit because it is trivially fixable and worth catching before merge, but not a runtime bug.