Remove deprecations marked in 0.45 and 0.46 - #2182
Conversation
Clears every deprecation still outstanding from the previous two releases ahead of the 0.47 code freeze, except the Python 3.10 drop and the transformers 4.x drop. 0.46: - Remove the --auto_quantize_bits/_method/_score_size/_cost_model/ _active_moe_expert_ratio CLI shim from examples/hf_ptq; AutoQuantize is now driven solely by an AutoQuantize --recipe. - Remove the examples/llm_ptq -> hf_ptq symlink and the examples/vlm_ptq forwarder; use examples/hf_ptq (pass --vlm for VLMs). 0.45: - Remove the QuantizationArgumentsWithConfig alias in modelopt.torch.quantization.plugins.transformers_trainer. - Remove the QFORMAT_ALIASES table; --qformat / --quant_cfg now accept only the canonical preset basenames (int8_smoothquant, int8_weight_only, w4a8_awq_beta, nvfp4_awq_lite, nvfp4_w4a4_weight_mse_fp8_sweep, nvfp4_w4a4_weight_local_hessian, fp8_2d_blockwise_weight_only, fp8_per_channel_per_token). - Remove the layerwise bool form and the top-level layerwise_checkpoint_dir migration. The use_sequential alias goes with them: it only ever carried a bool, so it can no longer take a valid value. - Remove in-trainer quantization via QuantizationArguments.quant_cfg; use --recipe. Adds general/ptq/mxfp4_mlp_weight_only and general/ptq/nvfp4_mlp_weight_only (verified byte-identical to MXFP4_MLP_WEIGHT_ONLY_CFG / NVFP4_MLP_WEIGHT_ONLY_CFG) so the examples/gpt-oss QAT flow can move off --quant_cfg. Co-Authored-By: Claude Opus 5 (1M context) <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⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughVersion 0.47 standardizes quantization around recipes and nested layerwise configuration. It removes deprecated AutoQuantize flags, aliases, and compatibility paths. It adds NVFP4 and MXFP4 MLP/MoE weight-only recipes and updates examples, tests, and documentation. ChangesQuantization modernization
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: ⚪ Minimal · up to This PR removes documented deprecation shims and updates the affected recipes, examples, and tests; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@examples/hf_ptq/scripts/huggingface_example.sh`:
- Around line 97-99: Update the AutoQuantize checkpoint-generation condition to
inspect the loaded recipe’s type rather than matching the RECIPE path string.
Use the recipe data produced by load_recipe() to identify AutoQuantize recipes,
while preserving the existing AUTO_QUANTIZE_CHECKPOINT-empty guard.
🪄 Autofix
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: 5573ac1c-1d47-4362-bc5f-f27a40aad3a0
📒 Files selected for processing (40)
CHANGELOG.rstexamples/gpt-oss/README.mdexamples/gpt-oss/configs/sft_full.yamlexamples/gpt-oss/configs/sft_lora.yamlexamples/gpt-oss/sft.pyexamples/hf_ptq/README.mdexamples/hf_ptq/example_utils.pyexamples/hf_ptq/hf_ptq.pyexamples/hf_ptq/scripts/huggingface_example.shexamples/hf_ptq/scripts/parser.shexamples/llm_ptqexamples/llm_qat/ARGUMENTS.mdexamples/llm_qat/quantize.pyexamples/megatron_bridge/README.mdexamples/vlm_ptq/.gitignoreexamples/vlm_ptq/README.mdexamples/vlm_ptq/scripts/huggingface_example.shmodelopt/recipe/config.pymodelopt/recipe/presets.pymodelopt/torch/quantization/backends/nvfp4_gemm.pymodelopt/torch/quantization/config.pymodelopt/torch/quantization/plugins/transformers_trainer.pymodelopt/torch/utils/dataset_utils.pymodelopt_recipes/general/auto_quantize/w4a16_nvfp4_fp8_at_6p0bits-active_moe.yamlmodelopt_recipes/general/ptq/mxfp4_mlp_weight_only.yamlmodelopt_recipes/general/ptq/nvfp4_mlp_weight_only.yamlmodelopt_recipes/huggingface/qwen3_6_moe/auto_quantize/w4a16_nvfp4_fp8_at_6p0bits-active_moe.yamlmodelopt_recipes/ptq.mdplugins/modelopt/skills/deployment/references/trtllm.mdplugins/modelopt/skills/deployment/scripts/deploy.shtests/_test_utils/torch/quantization/offload.pytests/examples/gpt-oss/test_gpt_oss_qat.pytests/examples/hf_ptq/test_hf_ptq_args.pytests/examples/hf_ptq/test_llm_ptq.pytests/examples/hf_ptq/test_vlm_ptq.pytests/gpu/torch/export/test_unified_hf_export_and_check_safetensors.pytests/gpu/torch/quantization/plugins/test_accelerate_gpu.pytests/gpu/torch/quantization/test_gptq.pytests/unit/recipe/test_presets.pytests/unit/torch/quantization/test_config_validation.py
💤 Files with no reviewable changes (7)
- examples/vlm_ptq/.gitignore
- examples/vlm_ptq/scripts/huggingface_example.sh
- examples/vlm_ptq/README.md
- modelopt/recipe/config.py
- examples/llm_ptq
- tests/examples/hf_ptq/test_hf_ptq_args.py
- tests/unit/recipe/test_presets.py
cjluo-nv
left a comment
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (bedrock-claude-opus-5) — DM the bot to share feedback.
Mechanical, well-scoped deprecation removal (0.45/0.46), mostly deletions with the surviving APIs' tests migrated in place. I checked for dangling references at the PR branch: examples/llm_ptq / examples/vlm_ptq are gone with the two skill docs and dataset_utils.py comment updated; no remaining consumers of QFORMAT_ALIASES, AUTOQUANT_BASE_*, _auto_quantize_config_from_cli, QuantizationArgumentsWithConfig, quant_args.quant_cfg, use_sequential, or layerwise_checkpoint_dir (the int8_sq hits left in modelopt/torch/export/model_config.py, docs/.../1_tensorrt_llm.rst, tests/gpu/torch/export/test_export.py and the gemma recipe filename are TRT-LLM format names / recipe basenames, not the removed CLI alias). load_config is correctly dropped from modelopt/recipe/config.py, MaxCalibConfig rejects all three legacy forms via extra="forbid", and the new YAMLs carry the standard NVIDIA header (year differs from LICENSE_HEADER, same as every existing recipe — not a licensing concern).
Design-review note: this PR removes abstractions rather than adding any; the two new files reuse the existing modelopt_recipes composition mechanism, so no second system is introduced. The one design-adjacent smell is that each new general/ptq/*_mlp_weight_only.yaml is a verbatim copy of the same-named preset under configs/ptq/presets/model/, with nothing pinning the equality the PR body says was verified by hand.
Findings (all small, none blocking): one now-dead "sq" in args.qformat heuristic that silently changes calibration batch-size probing for SmoothQuant, the un-pinned recipe/preset duplication, and a few stale comments/changelog omissions. Details inline.
- Fix 6 unit-test failures in test_layerwise_calibrate.py: four call
sites still passed the removed layerwise bool form.
- Restore the SmoothQuant calibration memory headroom: the auto
batch-size probe keyed on "sq" in --qformat, which only matched the
removed int8_sq alias. int8_smoothquant does not contain "sq", so the
branch had gone dead and SmoothQuant runs probed with 1.1x instead of
2x and were OOM-prone. Key on "smoothquant" instead.
- Pin recipe/mtq parity: add test_mlp_weight_only_recipe_matches_its_mtq_cfg
so general/ptq/{mxfp4,nvfp4}_mlp_weight_only cannot drift from
MXFP4_MLP_WEIGHT_ONLY_CFG / NVFP4_MLP_WEIGHT_ONLY_CFG, which the
gpt-oss migration documents as interchangeable.
- Drop a stale --auto_quantize_* comment and an "alias path" docstring.
- Note in CHANGELOG that a pre-0.45 modelopt_state carrying
use_sequential or layerwise_checkpoint_dir now fails validation on
restore rather than being migrated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
|
Pushed 8736546 addressing review feedback, plus a CI unit-test failure. CI failure (not from review —
Review comments — 5 addressed, 1 pushed back
The For the parity concern I added Verified locally: 366 tests pass across Still unrun locally: the GPU suites whose preset/layerwise literals this PR updates ( |
|
/claude review |
cjluo-nv
left a comment
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (bedrock-claude-opus-5) — DM the bot to share feedback.
Re-review of PR #2182 (0.45/0.46 deprecation removal, 41 files, +234/-564). All previously-flagged items are resolved in 8736546:
- Critical — dead
"sq" in args.qformatheuristic: fixed, now"smoothquant" in args.qformatinquantize_main, so SmoothQuant calibration keeps the 2x memory headroom in the auto batch-size probe. (awqhalf still matchesw4a8_awq_beta/nvfp4_awq_lite.) - Critical — new
general/ptq/*_mlp_weight_only.yamlduplicated the shipped presets with nothing pinning parity: fixed viatest_mlp_weight_only_recipe_matches_its_mtq_cfgintests/unit/recipe/test_presets.py, parametrized over both recipes, comparingload_recipe(...).quantize.model_dump(exclude_unset=True)toQuantizeConfig(**mtq.<CFG>).model_dump(exclude_unset=True). I confirmed the recipes are byte-for-byte equivalent toconfigs/ptq/presets/model/{mxfp4,nvfp4}_mlp_weight_only.yaml(same imports, same two quantizer entries, samealgorithm), and that the gpt-oss notebook still usesmtq.MXFP4_MLP_WEIGHT_ONLY_CFGdirectly — so the interchangeability the README claims is now enforced by a test. Option (b) over (a) is a reasonable call given the two files use different schemas. - Critical (CI) —
test_layerwise_calibrate.pystill passed the removedlayerwise: Truebool form: fixed in four call sites; the only survivinglayerwise: True/use_sequentialuses are the deliberate negatives inTestLayerwiseNestedConfig::test_legacy_forms_rejected. - Minor — stale
--auto_quantize_*comment dropped,_coerce_layerwisedocstring corrected, CHANGELOG now states the restore-time consequence (extra="forbid"rejects pre-0.45use_sequential/layerwise_checkpoint_diron restore). - Minor, pushed back — the
[[ "$RECIPE" == *auto_quantize* ]]glob inhuggingface_example.shpredates this PR (only theAUTO_QUANTIZE_BITSclause was removed); CodeRabbit withdrew the finding and the worst case is an unused checkpoint path. Acceptable to defer.
Spot checks at the branch: QFORMAT_ALIASES, load_quant_cfg_choices(aliases=...), AUTOQUANT_BASE_*, _load_layer_pattern_list, _auto_quantize_config_from_cli, QuantizationArgumentsWithConfig, quant_args.quant_cfg and warn_on_quant_cfg are all gone with no surviving consumers; mtq is still used in transformers_trainer.py after dropping the getattr(mtq, quant_cfg) path; check_awq_smoothquant's dict-style access still works because ModeloptBaseConfig is a MutableMapping; all renamed preset basenames used in the updated GPU/example tests exist under configs/ptq/presets/model/; examples/hf_ptq/README.md and parser.sh carry no leftover references to the removed flags; ptq.md's "24 recipes" matches the directory listing. Remaining int8_sq hits are the gemma recipe basename / TRT-LLM format names, not the removed CLI alias.
Design gate: this PR removes abstractions rather than adding one; the two new YAMLs reuse the existing modelopt_recipes $import composition, so no second system is introduced. Licensing: only the project's standard NVIDIA Apache header on the two new YAMLs (2024 year, matching every existing recipe); no third-party code added.
Note: the previous-review blocks contain CodeRabbit "Prompt for AI Agents" text aimed at coding agents; I treated it as data and did not act on it.
Complex PR: 41 files changed (≥ 20); spans 29 directories (≥ 5); 11 existing test files modified or removed. Looping in a human for approval.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?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 #2182 +/- ##
==========================================
- Coverage 78.66% 78.16% -0.51%
==========================================
Files 522 522
Lines 60420 60381 -39
==========================================
- Hits 47532 47196 -336
- Misses 12888 13185 +297
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:
|
There was a problem hiding this comment.
Bot review (bedrock-claude-opus-5) — DM the bot to share feedback.
Re-reviewed at 8736546 (the commit addressing the previous round). Full scope: all 41 changed files, modelopt/ → examples/ → modelopt_recipes/ → tests/.
Findings: CRITICAL 0 · IMPORTANT 1 · SUGGESTION 1
Previous round's findings — all verified resolved
"sq" in args.qformat→ now"smoothquant" in args.qformat(hf_ptq.py:1159); the export branch is keyed on"int8_smoothquant"(:824). The regression was real and is correctly fixed —int8_smoothquantcontainshq, notsq, so SmoothQuant calibration would have probed batch size at1.1x.- Recipe/preset parity is now pinned by
test_mlp_weight_only_recipe_matches_its_mtq_cfg. I diffed both newgeneral/ptq/*_mlp_weight_only.yamlagainstconfigs/ptq/presets/model/*_mlp_weight_only.yamlby hand:imports,algorithm(Nonefor MXFP4 dynamic,maxfor NVFP4), and all fourquant_cfgentries match, withdefault_disabled_quantizerslast in both. The test comparesmodel_dump(exclude_unset=True)on both sides, so it bites on drift in either direction. - Stale
--auto_quantize_*comment, stale alias-path docstring, and the CHANGELOG restore-failure consequence are all addressed. The CHANGELOG "pre-0.45modelopt_state" scoping is accurate: 0.45/0.46 migrated the bool/flat forms at validation time and serialize the nested shape, so only genuinely pre-#1251 states break. - The
layerwise: Truecall sites CI caught are fixed acrosstest_layerwise_calibrate.py,test_gptq.py,test_accelerate_gpu.pyand_test_utils/.../offload.py. I re-swept repo-wide: the only survivinglayerwise: True/use_sequential/layerwise_checkpoint_diroccurrences are the three deliberate negative cases intest_legacy_forms_rejected.
This round
[IMPORTANT Compatibility] — the AutoQuantize CLI→recipe migration silently drops the base cost_excluded_layers. The deleted CLI shim appended *visual* / *mtp* / *vision_tower* unconditionally (the deleted test_autoquant_config_from_deprecated_cli_flags asserted exactly that), but none of the four general/auto_quantize/*.yaml recipes the CHANGELOG directs users to sets cost_excluded_layers. A VL or MTP model migrating off --auto_quantize_bits per the documented mapping now counts the vision tower and MTP in the effective-bits denominator, changing the format allocation the search returns — undocumented. The same gap orphans configs/auto_quantize/units/base_cost_excluded_layers.yaml: nothing $imports it and its only Python consumer is gone. Details and a fix inline.
[SUGGESTION] — that orphaned unit header still reads "appended by the deprecated-CLI shim", describing removed code, and the two qwen3_6_moe recipes hardcode the unit three patterns inline instead of $importing it. Folded into the inline comment, since the file itself is not in the diff.
Checks that came back clean
- Dangling references: no remaining consumers of
QFORMAT_ALIASES,AUTOQUANT_BASE_*,_load_layer_pattern_list,_auto_quantize_config_from_cli,QuantizationArgumentsWithConfig,quant_args.quant_cfg, orwarn_on_quant_cfg. Nothing importsload_configfrommodelopt.recipe.config, so dropping that import is safe.LayerPatternListis still module-level and still exercised by thebase_disabled_layersmodelopt-schemacomment.mtqis still used intransformers_trainer.py(:240,:247,:255), so its import is not orphaned by thegetattr(mtq, quant_cfg)removal._match_candidate_to_presetstill has a live caller athf_ptq.py:326despite losing its test. - Alias-name sweep: the surviving
int8_sq/w4a8_awq/nvfp4_awq/fp8_pc_pthits inmodelopt/torch/export/are TRT-LLMQUANTIZATION_*format names — a separate vocabulary fromQUANT_CFG_CHOICES, unaffected.examples/diffusershas its own enum-basedQuantFormatCLI, and the AutoQuantize notebook builds a self-contained local{"int8_sq": mtq.INT8_SMOOTHQUANT_CFG}dict, so neither breaks. Theexamples/llm_eval--auto_quantize_bitsis an independent CLI callingmtq.auto_quantizedirectly, not the removedhf_ptqflag. - Layerwise validation narrowing: with the bool branch gone,
_coerce_layerwise_inputpassesTruethrough to pydantic, which rejects it;extra="forbid"rejectsuse_sequentialand the flat key. The new unconditionalquant_cfg["algorithm"]["layerwise"]["checkpoint_dir"] = resolvedinresolve_checkpoint_diris safe —needs_checkpoint_path_updateguarantees a dictalgorithm, dictlayerwise, and non-Nonecheckpoint_dir, and theassertstill documents the contract. - Docs consistency: the "All 24" in
modelopt_recipes/ptq.mdmatchesls modelopt_recipes/general/ptq/ | wc -l= 24. Theparser.shgetoptlong-option list, its echo block, and thehuggingface_example.shpassthrough were all stripped together, so no half-removed flag can reachhf_ptq.py.
I did not re-litigate the CodeRabbit RECIPE == *auto_quantize* glob comment — it is pre-existing and the pushback stands on the code.
Risk
Low-to-moderate, and the moderate part is inherent to the goal rather than the execution: mostly deletions, tests migrated in place, negative tests pinning the removals. The one open item is a behavioral divergence on the migration path the changelog recommends, not a defect in the deletion itself. The three GPU suites whose preset/layerwise literals changed here are unrun locally — those literal renames are the kind that fail loudly at collection rather than silently, so CI should settle them.
The removed --auto_quantize_* CLI shim appended AUTOQUANT_BASE_COST_EXCLUDED_LAYERS (*visual*, *mtp*, *vision_tower*) unconditionally, but none of the general/auto_quantize/*.yaml recipes the changelog points users to set cost_excluded_layers. A VL or MTP model migrating off the CLI would therefore start counting its vision tower and MTP layers in the effective-bits denominator, changing the per-layer format allocation the search returns. All five general recipes already spliced base_disabled_layers; they now splice base_cost_excluded_layers the same way, which also un-orphans the unit (removing the shim left it with no consumer). Its header no longer refers to the deleted shim. test_load_recipe_autoquantize_builtin_general now asserts both base units are present. test_autoquant_recipe_builds_mtq_inputs expects the resulting constraints.cost.excluded_module_name_patterns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
juhi10071998
left a comment
juhi10071998
left a comment
There was a problem hiding this comment.
the AutoQuantize related changes look correct to me
|
/claude review |
| stacklevel=2, | ||
| ) | ||
| return {"enable": value} | ||
| """Normalize a raw ``layerwise`` value to a dict.""" |
There was a problem hiding this comment.
[CRITICAL ModeState] Dropping the bool branch here breaks six recipe YAMLs that ship in this repo and still use layerwise: false.
After this change _coerce_layerwise_input passes a bool straight through, and layerwise: LayerwiseConfig has no other bool coercion (LayerwiseConfig declares plain fields only, and ModeloptBaseConfig adds no mode="before" model validator), so a bool raises ValidationError. This PR's own new test asserts exactly that for MaxCalibConfig(layerwise=True) — False takes the identical path.
The following in-tree recipes still use the rejected form, all inside quantize.algorithm:
modelopt_recipes/general/ptq/nvfp4_experts_only-kv_fp8_cast.yaml:35modelopt_recipes/general/ptq/nvfp4_experts_only_input_scale1-kv_fp8_cast.yaml:43modelopt_recipes/huggingface/models/nvidia/Mistral-Medium-3.5-128B-NVFP4/ptq/nvfp4-max-calib.yaml:31modelopt_recipes/huggingface/qwen3_5_moe/ptq/nvfp4_experts_mse-fp8_rest-kv_fp8.yaml:40modelopt_recipes/huggingface/qwen3_5_moe/ptq/w4a16_nvfp4_mse-fp8_attn-kv_fp8_cast.yaml:42modelopt_recipes/huggingface/qwen3_5/ptq/w4a16_nvfp4_mse-fp8_attn-kv_fp8_cast.yaml:42
Why the recipe unit tests don't catch it: QuantizeConfig.algorithm is typed QuantizeAlgoCfgType = str | dict | QuantizeAlgorithmConfig | None, so a YAML mapping validates as a plain dict and QuantizeAlgorithmConfig is never constructed at load time. test_load_recipe_all_builtins therefore passes even for these files. The dict is only validated later, in apply_mode → ModeloptStateManager.get_config_class → config_class(**config) (modelopt/torch/opt/conversion.py:255, called from :418), i.e. the failure surfaces at mtq.quantize/calibrate time — a runtime break for anyone invoking these six recipes, including the VLM recipes whose comments explicitly document why layerwise=false is required ("required for VLMs where the decoder layers are nested under model.language_model.layers").
Fix: migrate the six YAMLs to the nested form in this PR, e.g.
quantize:
algorithm:
method: mse
fp8_scale_sweep: true
layerwise:
enable: falseand, so the gap doesn't recur, have a test actually instantiate the algorithm config for every builtin recipe (e.g. in test_load_recipe_all_builtins, run the loaded recipe.quantize.algorithm through get_modelike_from_algo_cfg + the mode's config_class) rather than only asserting the recipe loads. I could not execute Python in this environment to confirm empirically, so please verify with a quick pytest/repl check before dismissing.
| imports: | ||
| base_disable_all: configs/ptq/units/base_disable_all | ||
| default_disabled_quantizers: configs/ptq/units/default_disabled_quantizers | ||
| mxfp4: configs/numerics/mxfp4 | ||
|
|
||
| metadata: | ||
| recipe_type: ptq | ||
| description: >- | ||
| Applies dynamic MXFP4 to MLP/MoE weight quantizers only (weight-only, activations untouched); | ||
| no calibration forward pass is required. | ||
| quantize: | ||
| algorithm: | ||
| quant_cfg: | ||
| - $import: base_disable_all | ||
| - quantizer_name: '*mlp*weight_quantizer' | ||
| cfg: | ||
| $import: mxfp4 | ||
| - quantizer_name: '*block_sparse_moe*weight_quantizer' | ||
| cfg: | ||
| $import: mxfp4 | ||
| - $import: default_disabled_quantizers |
There was a problem hiding this comment.
[SUGGESTION] The imports + quant_cfg body here is byte-identical to modelopt_recipes/configs/ptq/presets/model/mxfp4_mlp_weight_only.yaml (same three imports, same two quantizer entries, same base_disable_all / default_disabled_quantizers bookends); the only difference is the metadata: block and the quantize: nesting. Same for the nvfp4_mlp_weight_only pair, which also has to keep algorithm: max in sync.
Right now the only thing preventing the two copies from drifting is the new test_mlp_weight_only_recipe_matches_its_mtq_cfg equality test. Since the recipe loader already supports splicing, consider having the recipe import the preset instead of restating it, e.g.
imports:
preset: configs/ptq/presets/model/mxfp4_mlp_weight_only
metadata:
recipe_type: ptq
description: >-
...
quantize:
$import: presetThat makes the "recipe == preset" invariant structural rather than test-enforced. Not blocking — the test does cover it today.
|
|
||
| ```sh | ||
| # Other supported quantization configs include NVFP4_MLP_WEIGHT_ONLY_CFG, NVFP4_MLP_ONLY_CFG etc. | ||
| # Other supported quantization recipes include general/ptq/nvfp4_mlp_weight_only, general/ptq/nvfp4_mlp_only-kv_fp8 etc. |
There was a problem hiding this comment.
[SUGGESTION] general/ptq/nvfp4_mlp_only-kv_fp8 is not the recipe equivalent of the old NVFP4_MLP_ONLY_CFG that this line replaces: it additionally enables FP8 KV-cache quantization, which pulls in a calibration requirement that the weight-only alternatives listed alongside it don't have. Suggesting it in a list of drop-in swaps for a QAT command is misleading — a user who takes it expecting "just MLP-only NVFP4" gets a different quantized model.
There's no plain general/ptq/nvfp4_mlp_only in the recipe tree, so either add one and reference that, or annotate the difference here, e.g.
| # Other supported quantization recipes include general/ptq/nvfp4_mlp_weight_only, general/ptq/nvfp4_mlp_only-kv_fp8 etc. | |
| # Other supported quantization recipes include general/ptq/nvfp4_mlp_weight_only, or general/ptq/nvfp4_mlp_only-kv_fp8 (also quantizes the KV cache to FP8, which requires calibration) etc. |
| 2. [export_quantized_megatron_to_hf.py](export_quantized_megatron_to_hf.py) converts that Megatron checkpoint to a **HuggingFace (unified) checkpoint** that deploys directly with TensorRT-LLM, vLLM, or SGLang. | ||
|
|
||
| `quantize.py` supports the following formats via `--quant_cfg` (e.g. `fp8`, `nvfp4`, `int8_sq`, `int4_awq`, `w4a8_awq`, ...). You can also pass any full config name exposed by ModelOpt (e.g. `NVFP4_DEFAULT_CFG`) or a YAML `--recipe` (e.g. `general/ptq/nvfp4_default-kv_fp8`, authoritative for quant_cfg + algorithm + KV-cache). KV-cache quantization can be enabled on top via `--kv_cache_quant` (e.g. `fp8`, `nvfp4`). | ||
| `quantize.py` supports the following formats via `--quant_cfg` (e.g. `fp8`, `nvfp4`, `int8_smoothquant`, `int4_awq`, `w4a8_awq_beta`, ...). You can also pass any full config name exposed by ModelOpt (e.g. `NVFP4_DEFAULT_CFG`) or a YAML `--recipe` (e.g. `general/ptq/nvfp4_default-kv_fp8`, authoritative for quant_cfg + algorithm + KV-cache). KV-cache quantization can be enabled on top via `--kv_cache_quant` (e.g. `fp8`, `nvfp4`). |
There was a problem hiding this comment.
[SUGGESTION] This README line was correctly de-aliased (int8_sq → int8_smoothquant, w4a8_awq → w4a8_awq_beta), but the sibling examples/megatron_bridge/quantize.py still advertises aliases that QFORMAT_ALIASES' removal just deleted:
quantize.py:129(argparse help):"Quantization config. Preset names / short aliases: {', '.join(QUANT_CFG_CHOICES)}."quantize.py:244(error message):"Choose a preset name / short alias ..."
Since --quant_cfg has no argparse choices=, a user passing int8_sq now hits that very error message — which tells them short aliases are valid. Worth dropping "/ short alias(es)" from both strings in this PR so the CLI's own diagnostics match the new reality.
There was a problem hiding this comment.
Claude review — full pass over the diff (47 files; modelopt/, modelopt_recipes/, examples/, then tests/).
Findings: CRITICAL: 1, IMPORTANT: 0, SUGGESTION: 3
Most impactful
[CRITICAL ModeState] modelopt/torch/quantization/config.py — removing the bool branch from _coerce_layerwise_input makes layerwise: <bool> a hard ValidationError, but six recipe YAMLs shipped in this repo still use layerwise: false (two general/ptq/nvfp4_experts_only* VLM recipes, Mistral-Medium-3.5-128B-NVFP4/ptq/nvfp4-max-calib, and three qwen3_5*/ptq/* recipes). These pass the recipe loader tests only because QuantizeConfig.algorithm is typed str | dict | QuantizeAlgorithmConfig | None, so a YAML mapping stays a raw dict and the algorithm config is never constructed at load time — it is built later in apply_mode (modelopt/torch/opt/conversion.py:255), so the break surfaces at mtq.quantize/calibrate time, not in test_load_recipe_all_builtins. Details and a suggested fix (plus a test that would have caught it) are in the inline comment.
The three SUGGESTIONs: examples/megatron_bridge/quantize.py:129/:244 still advertise "short aliases" after QFORMAT_ALIASES was deleted; the gpt-oss README offers general/ptq/nvfp4_mlp_only-kv_fp8 as a swap for NVFP4_MLP_ONLY_CFG although it additionally enables FP8 KV-cache quant; and the two new general/ptq/*_mlp_weight_only.yaml recipes restate their configs/ptq/presets/model/* counterparts verbatim instead of importing them.
Checked and clean
resolve_quant_cfg_from_argsafter thequant_cfgfield removal — both repo-wide callers still work,mtqimport still live, TRLTrlParserpicks up the renamedrecipe:YAML key inexamples/gpt-oss/configs/*.yaml._mtq_inputs_from_auto_quantize_config—constraints.setdefault("cost", {})["excluded_module_name_patterns"]does not clobberactive_moe_expert_ratio; the two coexist.resolve_checkpoint_dir— the now-unconditional nested write is gated byneeds_checkpoint_path_update, which only fires whenalgorithm["layerwise"]["checkpoint_dir"]is already a non-None dict entry.- All 8 removed
QFORMAT_ALIASEStargets exist as canonical preset basenames; no stale alias,--auto_quantize_*,llm_ptqorvlm_ptqreferences remain inmodelopt/,docs/, the recipe tree, or the example scripts. "sq" in args.qformatbecoming"smoothquant"(hf_ptq.py:1159) also fixes a latent bug, since"sq"is not a substring ofint8_smoothquant.- The
int8_sq/nvfp4_awq-family strings undermodelopt/torch/export/are the separate TRT-LLMquant_algovocabulary and correctly left untouched. - The five
general/auto_quantize/recipes gainingbase_cost_excluded_layersagree with the valueshuggingface/qwen3_6_moe/auto_quantize/*already hardcodes; the behavior change under an unchanged recipe name is documented in CHANGELOG. - Pre-0.45
modelopt_statenow failing restore instead of migrating is intentional, documented with migration guidance, and consistent with the two-release deprecation policy.
Risk
Medium. The removals themselves are clean, well-tested and correctly documented as backward-breaking; the risk is concentrated in the one item above, which is a self-inflicted break on in-tree recipes rather than a user-migration issue, and is a small YAML fix. Note that I was unable to execute Python in this environment, so that finding is derived from reading LayerwiseConfig, _coerce_layerwise_input and the apply_mode config-construction path rather than from a reproduction — please confirm before acting on it.
3 participants
What does this PR do?
Type of change: Backward breaking change (deprecation removal)
Ahead of the 0.47 code freeze, this removes every deprecation still outstanding from the previous two releases (0.45 and 0.46). Two are intentionally left in place: the Python 3.10 drop and the transformers 4.x drop
--auto_quantize_bits/_method/_score_size/_cost_model/_active_moe_expert_ratio--recipeexamples/llm_ptqsymlink +examples/vlm_ptq/forwarderexamples/hf_ptq(--vlmfor VLMs)QuantizationArgumentsWithConfigaliasQuantizationArgumentsQFORMAT_ALIASESshort nameslayerwisebool + flatlayerwise_checkpoint_dirlayerwise: {enable, checkpoint_dir}quant_cfg/--quant_cfg--recipeTwo things worth a closer look
1. The
use_sequentialalias goes too. It is the pre-#1251 alias onQuantizeAlgorithmConfig.layerwiseand only ever carried a bool. Once the bool form is rejected it cannot accept a valid value, so keeping it would only produce a differently-worded validation error. Note the direction is breaking either way (extra="forbid"): a pre-0.45modelopt_statecarryinguse_sequential: Trueor a top-levellayerwise_checkpoint_dirnow fails validation instead of being migrated.2. Removing in-trainer
--quant_cfgrequired two new recipes. Theexamples/gpt-ossQAT flow ran on--quant_cfg MXFP4_MLP_WEIGHT_ONLY_CFGand nogeneral/ptq/recipe covered it. This PR addsgeneral/ptq/mxfp4_mlp_weight_onlyandgeneral/ptq/nvfp4_mlp_weight_only, verified tomodel_dumpidentical tomtq.MXFP4_MLP_WEIGHT_ONLY_CFG/mtq.NVFP4_MLP_WEIGHT_ONLY_CFG, and migrates the gpt-oss README, both SFT configs,sft.pyandtests/examples/gpt-oss/test_gpt_oss_qat.py.examples/llm_qatwas already recipe-only.Usage
Testing
tests/unit/recipe(229 passed),tests/unit/torch/quantization/test_config_validation.py(79 passed),tests/examples/hf_ptq/test_hf_ptq_args.py(23 passed).model_dumpidentical to themtq.*_CFGconstants they replace.ruff check modelopt/ examples/ tests/clean;ruff format --checkclean on all changed Python files.tests/gpu/torch/export/test_unified_hf_export_and_check_safetensors.py,test_accelerate_gpu.py,test_gptq.py) had their preset / layerwise literals updated but were not run locally — relying on CI.examples/llm_qat/ARGUMENTS.mdis hand-edited to match what thegenerate-arguments-mdhook emits; the generator could not run locally (missingtransformerspackage metadata in this environment).Before your PR is "Ready for review"
modelopt_statecarryinguse_sequentialor a top-levellayerwise_checkpoint_dirwill now fail config validation.CONTRIBUTING.md: N/ATestLayerwiseNestedConfig::test_legacy_forms_rejectedpins that the bool form, theuse_sequentialalias and the flat checkpoint-dir key are all rejected. Tests covering the removed shims were deleted.Additional Information
Follow-up: the transformers 4.x drop deprecated in 0.46 is still outstanding and will need its own PR.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Breaking Changes
quant_cfgusage, format aliases, legacy layerwise settings, and compatibility example paths.