Skip to content

Fix TP6 virtual padding for GLM MTP drafts - #96

Closed
voipmonitor wants to merge 1 commit into
dev/fathomless-firmamentfrom
codex/fathomless-firmament-v17-tp6-mtp-fix-20260715
Closed

Fix TP6 virtual padding for GLM MTP drafts#96
voipmonitor wants to merge 1 commit into
dev/fathomless-firmamentfrom
codex/fathomless-firmament-v17-tp6-mtp-fix-20260715

Conversation

@voipmonitor

Copy link
Copy Markdown

Summary

  • restore virtual-TP padding for independently constructed MTP draft model configs
  • apply the draft padding plan before draft parallel-config validation
  • add a regression test that verifies padding precedes the divisibility check

Root cause

The GLM target config was padded from 64 to 66 attention heads in VllmConfig.__post_init__, but SpeculativeConfig validates the independently constructed DeepSeekMTPModel first. The v17 stack contained the draft-padding test but omitted the corresponding helper and validator call, so TP6 worked with MTP off and failed with MTP enabled.

Validation

  • 19 passed in tests/config/test_virtual_tp.py
  • Ruff check and format check passed
  • exact config-only reproduction now passes for TP6, DCP6, MTP3 with both target and draft at 66 heads / 2112 MoE intermediate size

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • dev/*

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 751da9f4-c1ba-4183-9cfe-72f06853d6b8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fathomless-firmament-v17-tp6-mtp-fix-20260715

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@voipmonitor
voipmonitor force-pushed the codex/fathomless-firmament-v17-tp6-mtp-fix-20260715 branch from 05f50ae to 522c626 Compare July 15, 2026 16:23
@voipmonitor
voipmonitor changed the base branch from codex/fathomless-firmament-v17-dcp-prefill-opt-20260714 to dev/fathomless-firmament July 15, 2026 16:23
@voipmonitor

Copy link
Copy Markdown
Author

Validated after rebasing directly onto dev/fathomless-firmament:

  • PR head: 522c626de89b629a18d05db21ba02b5acf6e6f30
  • focused tests: 19 passed
  • real lukealonso/GLM-5.2-NVFP4 config at TP6/DCP6/MTP3: target and draft both resolve to 66 attention heads / 2112 MoE intermediate size
  • exact-v17 source image carrying the equivalent fix: voipmonitor/vllm:fathomless-firmament-v17-vllm05f50ae-b12x1377d5f-fi801d57a-cu132-20260715
  • manifest: sha256:9b6f1ab6db4d3a7b7b786481eb32abe82e86d185648d62c3ac1cfa6d72a55e47

The release uses commit 05f50ae79c48835275f22f76e8dfb10b0024dec6 only to retain the exact v17 integration parent; the mergeable fix remains this single direct-FF PR.

@voipmonitor

Copy link
Copy Markdown
Author

/review

@voipmonitor

Copy link
Copy Markdown
Author

Superseded by d437a23cfd (Add reusable virtual TP sharding for odd TP sizes) already present on dev/fathomless-firmament. The canonical implementation is broader (MTP and DSpark) and includes the TP6 draft-before-validation regression coverage from this PR. Closing to keep the FF merge queue unambiguous.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant