Skip to content

dsv4-fp8-h200-vllm-mtp: bump num_speculative_tokens 1 → 2 #1279

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

Merged
functionstackx merged 2 commits into from
May 5, 2026
+9 −2
Merged

dsv4-fp8-h200-vllm-mtp: bump num_speculative_tokens 1 → 2 #1279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
562ac10
dsv4-fp8-h200-vllm-mtp: bump num_speculative_tokens 1 → 2
functionstackx May 4, 2026
db615ed
perf-changelog: fill in PR #1279 link
functionstackx May 4, 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
4 changes: 2 additions & 2 deletions benchmarks/single_node/dsv4_fp8_h200_mtp.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# DeepSeek-V4-Pro H200 vLLM MTP variant of the recipe at
# https://vllm.ai/blog/deepseek-v4. Mirrors dsv4_fp8_h200.sh but adds
# --speculative-config '{"method":"mtp","num_speculative_tokens":1}' and
# --speculative-config '{"method":"mtp","num_speculative_tokens":2}' and
# routes prompts through chat-formatted encoding via --dsv4 (required for
# meaningful MTP acceptance numbers per AGENTS.md).

Expand Down Expand Up @@ -65,7 +65,7 @@ $MAX_MODEL_LEN_ARG \
--max-num-batched-tokens 512 \
--no-enable-flashinfer-autotune \
--compilation-config '{"mode":0,"cudagraph_mode":"FULL_DECODE_ONLY"}' \
--speculative-config '{"method":"mtp","num_speculative_tokens":1}' \
--speculative-config '{"method":"mtp","num_speculative_tokens":2}' \
--tokenizer-mode deepseek_v4 \
--tool-call-parser deepseek_v4 \
--enable-auto-tool-choice \
Expand Down
7 changes: 7 additions & 0 deletions perf-changelog.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2207,3 +2207,10 @@
- "run_benchmark_serving uses --dsv4 (chat-formatted prompts) per the AGENTS.md MTP rule, since EAGLE-style speculative decoding regresses acceptance on raw random tokens"
- "Search space mirrors the non-MTP H200 entry: TP=8, EP=8, DP-attn=true, CONC 4-64 for both 1k1k and 8k1k, with spec-decoding: mtp"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1222

- config-keys:
- dsv4-fp8-h200-vllm-mtp
description:
- "Bump --speculative-config num_speculative_tokens from 1 to 2 (`{\"method\":\"mtp\",\"num_speculative_tokens\":2}`)"
- "Re-test whether H200 MTP kernels accept 2 draft tokens — Blackwell MTP runs at 2 (per @wzhao18's vLLM Blackwell MTP submission); checking if H200 has parity now"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1279
Copy link
Copy Markdown
Contributor

@claude claude Bot May 4, 2026

Choose a reason for hiding this comment

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

🔴 The new perf-changelog.yaml entry for this PR has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER on line 2216 — the seed placeholder from the AGENTS.md template was never replaced with the real PR number (1279). Clicking through the changelog entry to find the originating PR returns a GitHub 404, defeating the audit-trail purpose of the file. Trivial one-token fix on line 2216: PLACEHOLDER → 1279.

Extended reasoning...

What the bug is

The new entry appended to perf-changelog.yaml for the dsv4-fp8-h200-vllm-mtp sweep ends with:

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

The rendered diff text in the GitHub UI happens to show pull/1279, but the actual committed content at HEAD (026143c) on line 2216 still says PLACEHOLDER. tail -1 perf-changelog.yaml and git show HEAD -- perf-changelog.yaml | tail -1 both confirm this directly.

Why the placeholder is there

AGENTS.md (around lines 281 and 304) instructs authors to seed new changelog entries with pull/XXX-style placeholder text and substitute the real PR number after pushing. The author of this PR followed the template but skipped the substitution step.

Why nothing caught it

utils/matrix_logic/validation.py:483 declares pr_link as a plain str, with no URL/format/regex constraint:

pr_link: str

So PLACEHOLDER validates fine. There is no other test or lint that would have rejected it.

How rare this is

This is the only non-numeric pr-link in the entire 252-entry file:

$ grep -n 'pr-link' perf-changelog.yaml | grep -v 'pull/[0-9]'
2216:  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER

Every other one of the ~250 entries points at a real numeric PR.

Step-by-step proof

  1. Reviewer opens perf-changelog.yaml at HEAD after this PR merges.
  2. They scroll to the bottom (line 2216) — the entry for dsv4-fp8-h200-vllm-mtp sweep [NVIDIA] Add TRT 70B (FP8 and FP4) #2.
  3. They click the pr-link to find the PR that introduced this sweep.
  4. Browser navigates to https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER.
  5. GitHub returns a 404 — PLACEHOLDER is not a numeric PR id.
  6. The reviewer cannot trace the perf delta back to the originating PR via the changelog, which is precisely the workflow the changelog exists to support.

Impact

Not a runtime/behavior bug — the benchmark recipe itself is untouched. But the changelog entry is broken-as-merged: the audit-trail link 404s, and the entry sits next to ~250 well-formed siblings as the lone outlier. Easy to miss once it lands on main.

Fix

One-token edit on line 2216 of perf-changelog.yaml:

-  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/PLACEHOLDER
+  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1279

(Optional follow-up, out of scope for this PR: tighten pr_link in utils/matrix_logic/validation.py:483 to require a numeric PR id, e.g. pydantic.HttpUrl + a regex check or a custom validator that asserts the path ends with /pull/<int>. That would prevent the same template-substitution miss from sneaking past in the future.)

Loading