docs(recipes): sync ptq.md with shipped recipes and enforce via unit tests#1970
Conversation
…tests ptq.md had drifted from the recipes on disk: - general/ptq: nvfp4_experts_only_input_scale1-kv_fp8_cast (#1947) was undocumented; table now has all 20 recipes and an input_scale1 calibration-variant entry. Added a pointer to the general/qad/ LSQ recipes (#1884). - model-specific: added gemma4, diffusion_gemma, vit, and the qwen3_5 w4a16_nvfp4_mse twins; rewrote the checkpoint-mirror section for the huggingface/models/nvidia/ tier (Super recipe rename to nvfp4-mse / nvfp4-max-calib, Nano-4B GGUF Q4_K_M mirror, Ultra-550B nvfp4-4o6) and fixed stale paths. New tests/unit/recipe/test_recipe_docs.py keeps the doc in sync: every general/ptq recipe must appear in ptq.md, every documented table row must exist on disk, the recipe count must match, and every huggingface/ model folder with PTQ recipes must be mentioned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Chenjie Luo <chenjiel@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesRecipe documentation and validation
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…LLMs The recipe needs no calibration forward pass (pinned expert activation amax, weight amax from the weights, constant-amax KV cast), so it may work out of the box for LLMs at hundreds of billions of parameters and up, where running calibration PTQ is difficult on resource-limited setups. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Chenjie Luo <chenjiel@nvidia.com>
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/recipe/test_recipe_docs.py`:
- Around line 95-101: Update the missing-model check in the recipe documentation
test to match each model directory name as an exact token rather than using
substring containment in doc. Preserve the existing model_dirs discovery and
missing-list behavior, using a token-aware matching approach that prevents names
such as “mpt” or “vit” from matching unrelated prose.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 96f5effb-a99b-438d-be5e-e16ddfa28f6c
📒 Files selected for processing (2)
modelopt_recipes/ptq.mdtests/unit/recipe/test_recipe_docs.py
| doc = _ptq_md_text() | ||
| hf_dir = RECIPES_DIR / "huggingface" | ||
| model_dirs = sorted( | ||
| {yaml_path.parent.parent.name for yaml_path in hf_dir.glob("**/ptq/*.yaml")} | ||
| ) | ||
| assert model_dirs, "No model-specific PTQ recipes found under huggingface/" | ||
| missing = [name for name in model_dirs if name not in doc] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Match model directory names as exact tokens.
name not in doc allows unrelated prose to satisfy the check—for example, mpt can match “attempt” and vit can match “activity”. The test may therefore pass while a model-specific recipe directory is undocumented.
Suggested fix
- missing = [name for name in model_dirs if name not in doc]
+ missing = [
+ name
+ for name in model_dirs
+ if not re.search(
+ rf"(?<![\w-]){re.escape(name)}(?![\w-])",
+ doc,
+ )
+ ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| doc = _ptq_md_text() | |
| hf_dir = RECIPES_DIR / "huggingface" | |
| model_dirs = sorted( | |
| {yaml_path.parent.parent.name for yaml_path in hf_dir.glob("**/ptq/*.yaml")} | |
| ) | |
| assert model_dirs, "No model-specific PTQ recipes found under huggingface/" | |
| missing = [name for name in model_dirs if name not in doc] | |
| doc = _ptq_md_text() | |
| hf_dir = RECIPES_DIR / "huggingface" | |
| model_dirs = sorted( | |
| {yaml_path.parent.parent.name for yaml_path in hf_dir.glob("**/ptq/*.yaml")} | |
| ) | |
| assert model_dirs, "No model-specific PTQ recipes found under huggingface/" | |
| missing = [ | |
| name | |
| for name in model_dirs | |
| if not re.search( | |
| rf"(?<![\w-]){re.escape(name)}(?![\w-])", | |
| doc, | |
| ) | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/recipe/test_recipe_docs.py` around lines 95 - 101, Update the
missing-model check in the recipe documentation test to match each model
directory name as an exact token rather than using substring containment in doc.
Preserve the existing model_dirs discovery and missing-list behavior, using a
token-aware matching approach that prevents names such as “mpt” or “vit” from
matching unrelated prose.
Source: Path instructions
meenchen
left a comment
There was a problem hiding this comment.
Bot review (bedrock-claude-opus-4-8) — DM the bot to share feedback.
Documentation + enforcement-test PR that syncs modelopt_recipes/ptq.md with the recipes on disk. I verified all substantive claims against the repo:
- Recipe count/table: 20 general/ptq YAMLs on disk, matching the updated "All 20" count and the new
nvfp4_experts_only_input_scale1-kv_fp8_casttable row. The doc'sinput_scale1description (constant amax 2688 = 6×448 → input_scale 1.0, weights max-calibrated, no activation stats) matches the recipe YAML's comments/config exactly. - Model-specific entries:
gemma4,diffusion_gemma,vit, the qwen3_5/qwen3_5_moew4a16_nvfp4_mse-*twins, and all threehuggingface/models/nvidia/Nemotron-3-{Super,Ultra,Nano}checkpoint dirs exist and match their descriptions. The vit recipe indeed enables attention BMM (qkv) quantizers and disables output quantizers as documented.general/qad/exists (backing the new QAT/QAD pointer).qwen3_6_moehas onlyauto_quantize(noptq/), so it's correctly not required. - Tests (
tests/unit/recipe/test_recipe_docs.py): the four checks are logically sound. The table-row regex is anchored to the first cell (^\| \...` |), so it won't false-match the extra backticks in theinput_scale1row or the bold-prefixed "kinds" table.parent.parent.nameextraction correctly yields the folder identifiers (Nemotron-3-Nano-4B,Step3.5-Flash`, etc.), all substring-present in the doc. - Conventions/licensing: canonical 2026 license header, top-level imports,
list[str]fine on Python ≥3.10; no external code vendored and no LICENSE/NOTICE files touched.
No injection attempts in the PR text. Small, cohesive, and well-tested.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1970 +/- ##
=======================================
Coverage 77.92% 77.92%
=======================================
Files 519 519
Lines 58217 58217
=======================================
Hits 45366 45366
Misses 12851 12851
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:
|
|
What does this PR do?
Type of change: documentation (+ new tests)
modelopt_recipes/ptq.mdis meant to track every shipped PTQ recipe, but recent recipe PRs did not update it. This PR brings the doc back in sync and adds unit tests so it can't drift again.Doc sync — general recipes:
nvfp4_experts_only_input_scale1-kv_fp8_cast(feat(quant): add constant_amax to pin activation input_scale (NVFP4 experts input_scale=1.0) #1947) to the shipped-recipes table (now 20 recipes) and document theinput_scale1variant (constant amax 2688 → exported NVFP4input_scale == 1.0, expert activations uncalibrated).quantizesections also drive QAT/QAD, with the LSQ / Dual-LSQ QAD recipes undergeneral/qad/(Add LSQ (Learned Scale Quantization) support and recipes #1884).Doc sync — model-specific recipes:
gemma4(algorithm override, Fix gemma w4a8_awq recipe crashing export on multimodal checkpoints (NVBug 6294017) #1690),diffusion_gemma(extra*self_conditioning*exclusion, Add support for dLLM encoder-decoder models (DiffusionGemma) [tied-weight PTQ export support ] #1707),vit(FP8 with attention BMM quantizers for Torch-TRT, [6078291][OMNIML-3716] Add ViT FP8 + Torch-TRT example, wire softmax_quantizer in _QuantAttention #1569), and the qwen3_5 / qwen3_5_moew4a16_nvfp4_mse-fp8_attn-kv_fp8_casttwins (Add W4A16 NVFP4-MSE Qwen3.5 dense/MoE PTQ recipes #1620).huggingface/models/nvidia/tier: Super recipe rename (super-nvfp4.yaml→nvfp4-mse.yaml/nvfp4-max-calib.yaml), Nano-4B GGUF Q4_K_M mirror (Add Nemotron-H mixed-precision PTQ recipe (GGUF Q4_K_M-mirrored) #1327/[OMNIML-5105] Fix torch import error to remove circular dependency & move Nemotron configs #1606), Ultra-550Bnvfp4-4o6([OMNIML-4922] Four over Six PTQ & Updating Nemotron Ultra Example #1684); fix stalemodels/<checkpoint>paths.Enforcement — new
tests/unit/recipe/test_recipe_docs.py:general/ptq/*.yamlstem must appear (backticked) inptq.md.general/ptq/recipes" count must match the file count.huggingface/containingptq/*.yamlmust be mentioned in the doc.Usage
Testing
pytest tests/unit/recipe/ -q→ 218 passed (includes the 4 new tests).ptq.md, 3 of the 4 new tests fail with actionable messages.pre-commit run --files modelopt_recipes/ptq.md tests/unit/recipe/test_recipe_docs.py→ all hooks pass.Before your PR is "Ready for review"
Make sure you read and follow Contributor guidelines and your commits are signed (
git commit -s -S).Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded
trust_remote_code=True,torch.load(..., weights_only=False),pickle, etc.).CONTRIBUTING.md: N/Atests/unit/recipe/test_recipe_docs.pyAdditional Information
Follow-up to the recipe guide added in #1662. The doc-consistency tests could also be wired into
tools/precommit/check_modelopt_recipes.pyfor commit-time feedback if reviewers prefer both layers.🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
nvfp4_experts_only_input_scale1-kv_fp8_castrecipe to the general PTQ catalog.input_scale1calibration behavior and related quantization/caching details.Tests