Repository navigation
Leave MTP layers unquantized at any depth in the default PTQ exclusions - #2911
Conversation
The default exclusions matched only a root-level mtp.* block, so Megatron vision-language models (language_model.mtp.*) had their MTP layers quantized by the general recipes, and the vLLM fake-quant export then refused the checkpoint. Match *mtp.* instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (2 skipped: 2 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: excluding MTP layers from default PTQ quantization at any nesting depth. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Security Anti-Patterns PASS. The pull request changes only CHANGELOG.rst, one YAML recipe, and a unit test. It adds no Python changes under modelopt or examples and no dependency-manifest changes. The added test contains no… Full details: Security Anti-Patterns
-
Explanation
PASS. The pull request changes only CHANGELOG.rst, one YAML recipe, and a unit test. It adds no Python changes under modelopt or examples and no dependency-manifest changes. The added test contains no torch.load, numpy.load(..., allow_pickle=True), trust_remote_code=True, external-input eval/exec, or # nosec pattern. SECURITY.md also excludes tests from these contributor rules.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2911 +/- ##
==========================================
+ Coverage 70.08% 78.82% +8.74%
==========================================
Files 659 659
Lines 74081 74081
==========================================
+ Hits 51917 58392 +6475
+ Misses 22164 15689 -6475
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
cjluo-nv
left a comment
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (gpt-6.1-sol) — DM the bot to share feedback.
Approve: the shared exclusion now matches root and nested MTP layers, and the regression test verifies decoder quantization remains enabled.
No action needed.
|
2 participants
What does this PR do?
Type of change: Bug fix
The default PTQ exclusions (
configs/ptq/units/default_disabled_quantizers, imported by thegeneral/ptq/*recipes andmtq.*_CFGpresets) disable multi-token-prediction layers withmtp.*, which only matches an MTP block at the model root (HF checkpoints, Megatron LLMs). Megatron vision-language models nest it under the language model (language_model.mtp.layers.0...), so their MTP layers were quantized. For Qwen3.5/3.6 VLMs quantized withexamples/megatron_bridge/quantize.pyand a general recipe,export_quantized_megatron_to_hf.py --vllm_fakequant_exportthen refuses the checkpoint ("MTP quantization is not supported by vLLM fakequant export/reload"). The pattern is now*mtp.*, matching the model-specific recipes that already use*mtp*.Models whose MTP block was nested and previously quantized by a general recipe now keep it in BF16, which is what the exclusion intends. Recipes that quantize MTP on purpose (e.g. the Nemotron 3.5 Super ones) do not import this unit after their MTP entries, so they are unaffected.
Usage
No API change.
Testing
tests/unit/recipe/test_presets.py::test_default_exclusions_disable_mtp_at_any_depth: withgeneral/ptq/fp8_default-kv_fp8, MTP quantizers at the root and underlanguage_model.are disabled and decoder quantizers stay enabled; fails without the change.tests/unit/recipe: 492 passed.nvfp4_default-kv_fp8checkpoint was refused onlanguage_model.mtp.layers.0.eh_proj.input_quantizer.Before your PR is "Ready for review"
CONTRIBUTING.md: N/AAdditional Information
Related to #2889 (vLLM fake-quant export through Megatron-Bridge).
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation