Remove deprecations marked in 0.45 and 0.46 - #2182
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 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 deprecated interfaces and updates documented replacements and tests; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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.
|
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.95% 78.36% -0.59%
==========================================
Files 522 522
Lines 60589 60550 -39
==========================================
- Hits 47840 47453 -387
- Misses 12749 13097 +348
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.
|
/claude review |
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.
9f79f17 to
ad331ce
Compare
|
Pushed The CRITICAL finding was correct and is the most serious bug this PR had. Six shipped PTQ recipes still used
Anyone running these six would have hit a Fixed all six, and closed the gap that hid them: Other three review comments — all taken:
Rebase: two Verified locally: 416 tests pass; Known-flaky, unrelated: the previous run failed |
|
/claude review |
There was a problem hiding this comment.
Claude review — full pass over the diff at ad331ce (54 files: modelopt/ → modelopt_recipes/ → examples/ → tests/; opened every source file, skipping only the deleted vlm_ptq docs and the two skill-doc string swaps).
Findings: CRITICAL: 0, IMPORTANT: 0, SUGGESTION: 2
Both suggestions are documentation-only and non-blocking: a stale layerwise=false comment in the two VLM recipes whose next line this PR rewrote, and the changelog not mentioning that presets.QFORMAT_ALIASES / load_quant_cfg_choices(aliases=...) went with the CLI short names.
Previous rounds' findings — verified resolved at ad331ce
layerwise: falsebool in six shipped recipes (last round's CRITICAL) is fixed. All six now carrylayerwise: {enable: false}, and I swept the whole recipe tree — every remaininglayerwiseblock is nested.LayerwiseConfig.enabledefaults toFalse, so the rewrite is behaviour-preserving:_coerce_layerwise_input(False)previously produced exactly{"enable": False}.- The gap that let it through is now closed by
test_shipped_ptq_recipe_algorithm_config_constructs, and closed the right way — it walksget_modelike_from_algo_cfgthenCalibrateModeRegistry[...].config_class(**cfg), the same pathapply_modetakes, so a malformedalgorithmblock fails at test time instead of insidemtq.quantize.get_modelike_from_algo_cfghandles all four shapes the shipped recipes use (None,str,dict,list). Discovery is complete rather than sampled: 49 YAMLs declarerecipe_type: ptqliterally and all 49 are collected (theconfigs/skip and theisinstance(raw, dict)guard only drop fragments and list-shaped units). cost_excluded_layerssilently dropped in the CLI-to-recipe migration (last round's IMPORTANT) is fixed. All fivegeneral/auto_quantize/*.yamlnow import the shared unit,test_load_recipe_autoquantize_builtin_generalasserts the patterns on every general recipe, and the mtq-input test pins the resultingconstraints. I traced it end to end:constraints.setdefault("cost", {})inhf_ptq.pydoes not clobberactive_moe_expert_ratioon the one recipe that sets both, and both cost models accept the exclusion key. The orphaned-unit header comment is corrected too.- Recipe/preset parity is pinned by
test_mlp_weight_only_recipe_matches_its_mtq_cfg, and the duplication that prompted that suggestion is gone entirely — the two new recipes are now three-line import wrappers overconfigs/ptq/presets/model/*, which is strictly better than the copy-plus-test I had suggested. Sincemtq.MXFP4_MLP_WEIGHT_ONLY_CFG/NVFP4_MLP_WEIGHT_ONLY_CFGare themselves loaded from those same presets, the equality is now structural rather than merely asserted.
This round — new ground covered
The whole-field import splice on quantize is a new loader pattern; no pre-existing recipe uses it, so I traced it rather than assuming. It resolves correctly: the loader derives schema_type=ModelOptPTQRecipe from metadata.recipe_type before import resolution, so _child_schema hands QuantizeConfig down to the import branch in config_loader.py; the preset carries the schema comment that is mandatory for anything in imports:, its own nested imports are resolved recursively, and the snippet is validated against QuantizeConfig before the merge. The absence of a schema header comment on the two new recipes is consistent — none of the 24 general/ptq/ recipes has one.
Also checked, clean
- Dead-import sweep after the deletions:
load_configcorrectly dropped frommodelopt/recipe/config.py(no remaining use),Mappingfrompresets.py,warningsfromtransformers_trainer.py(zero remaining calls) — whilequantization/config.pykeeps itswarningsimport, which one surviving call still needs.QUANT_CFG_CHOICESand_match_candidate_to_presetboth still have live users inhf_ptq.pyafter_auto_quantize_config_from_cliwas deleted. resolve_checkpoint_dirlosing its dual-shape handling is safe: the now-unconditional nestedcheckpoint_dirwrite is only reachable vianeeds_checkpoint_path_update, which requiresalgorithm["layerwise"]to be adictwith a non-Nonecheckpoint_dir.resolve_quant_cfg_from_argsafter thequant_cfgfield removal: both repo callers (QATTrainer,llm_qat/quantize.py) still work, and the return type is unchanged from the pre-existing recipe branch —QuantizeConfigis aMutableMapping, so the dict-style consumers downstream are unaffected. TRL'sTrlParserpicks up the renamedrecipe:key in bothexamples/gpt-oss/configs/*.yaml; both still leave it empty, as before.- The
sq-to-smoothquantsubstring fixes inhf_ptq.py(export branch and memory-ratio heuristic) are correct, and theawqhalf of the heuristic still matchesw4a8_awq_beta/nvfp4_awq_lite/int4_awq. - Dangling-reference sweep: no surviving consumer of
QFORMAT_ALIASES,AUTOQUANT_BASE_*,_load_layer_pattern_list,_auto_quantize_config_from_cli,QuantizationArgumentsWithConfig,quant_args.quant_cfg,use_sequential,layerwise_checkpoint_dir, or thellm_ptq/vlm_ptqpaths. The--auto_quantize_*hits remaining inexamples/llm_evalandexamples/alpamayoare those scripts own independent flags, correctly out of scope — the changelog scopes the removal toexamples/hf_ptq. The onlyuse_sequentialand bool-layerwiseuses left in the repo are the three deliberate negatives intest_legacy_forms_rejected. ptq.md's "All 24" matches the directory listing, and every renamed preset basename used in the updated GPU and example tests exists underconfigs/ptq/presets/model/.- The pre-0.45
modelopt_staterestore failure is intentional, and the changelog states both the consequence and the remedy.
I could not execute the test suite in this environment (the sandbox blocked pytest), so the verification above is static — CI is the gate on the four migrated GPU suites the author also did not run locally.
Risk: low. Mechanical deprecation removal, mostly deletions, with the two regressions found in earlier rounds now fixed and pinned by tests that exercise the real apply_mode path rather than just the loader. LGTM.
Edwardf0t1
left a comment
Edwardf0t1
left a comment
There was a problem hiding this comment.
LGTM - please resolve conflicts.
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 the 0.45/0.46 deprecation removal (54 files, +305/-580, mostly deletions). All previously-flagged critical findings are resolved in the current diff, and I verified the fixes rather than taking them on faith:
Verified resolved
"sq" in args.qformat→"smoothquant" in args.qformat(both the export branch athf_ptq.py:824and the calibration memory-ratio heuristic);int8_smoothquantreally does not containsq, so the branch was dead.- All six shipped recipes that used the removed bool form now carry
layerwise: {enable: false}, and the detection gap is closed bytest_shipped_ptq_recipe_algorithm_config_constructs, which discovers recipes from disk and walksget_modelike_from_algo_cfg→CalibrateModeRegistry[...].config_class(**cfg)— the same pathapply_modetakes, so this class of break now fails at test time instead of insidemtq.quantize. - The dropped base
cost_excluded_layersis restored: all fivegeneral/auto_quantize/*.yamlnow$importthe shared unit,test_load_recipe_autoquantize_builtin_generalasserts it, andtest_autoquant_recipe_builds_mtq_inputspins the resultingconstraints.cost. - The two new
general/ptq/*_mlp_weight_only.yamlare now three-line$importwrappers over the shipped presets; I traced the whole-field splice through_resolve_imports/_child_schemaand it resolves correctly (the presets carry the requiredmodelopt-schemacomment and their own nested imports resolve recursively). - Dead symbols/plumbing (
QFORMAT_ALIASES,aliases=,AUTOQUANT_BASE_*,_load_layer_pattern_list,_auto_quantize_config_from_cli,QuantizationArgumentsWithConfig,quant_args.quant_cfg,warn_on_quant_cfg,load_configimport, parser.sh flags/echoes) are gone with no surviving consumers; the remaining--auto_quantize_*hits areexamples/llm_eval/examples/alpamayo's independent CLIs. Changelog and docs (including the megatron_bridge help/error strings) are updated.
Why I'm flagging rather than approving — one open thread from a maintainer, plus scale:
- Edwardf0t1's comment on
general/auto_quantize/nvfp4_fp8_at_5p4bits.yaml:47(the newest review comment, on thecost_excluded_layerssplice) has no reply and no code change. My read is that it is already mitigated:_resolved_search_setup_signatureincludesfloat(hparam.cost_weight)per hparam, andmodule_cost_weightreturns0.0for modules matchingexcluded_module_name_patterns, so on a VL/MTP model — the only case where the new patterns change anything — resuming a stale checkpoint raises "Checkpoint resolved search setup does not match … cost weights. Use a different checkpoint path.", i.e. exactly the guard he asked for; on non-VL models the signature is unchanged and resume stays valid. Worth the author confirming that on the thread (ideally with a test) so the maintainer can close it, rather than me closing it by inference. - Minor, non-blocking: the two
huggingface/qwen3_6_moe/auto_quantize/*.yamlrecipes still hardcode the threecost_excluded_layerspatterns inline instead of$importing the now-shared unit, so the set has two definitions again; and the inline comment alignment inqwen3_5_moe/ptq/nvfp4_experts_mse-fp8_rest-kv_fp8.yamlis now off after the value grew. - Scale/CI: 54 files across 33 directories with 11 test files touched, and four GPU suites whose preset/layerwise literals changed were not run locally — those failures are loud rather than silent, but CI is the gate.
Design gate: this PR removes abstractions rather than adding any, and 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.
Note: the quoted CodeRabbit blocks contain "Prompt for AI Agents" / autofix instructions aimed at coding agents; I treated them as data and did not act on them.
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>
- 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>
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>
Six shipped PTQ recipes still used the removed `layerwise: false` bool
form. QuantizeConfig.algorithm accepts a bare dict, so load_recipe never
constructs QuantizeAlgorithmConfig and test_load_recipe_all_builtins
passed on all six; the ValidationError only surfaced later inside
mtq.quantize. Converted them to `layerwise: {enable: false}`.
Closes the detection gap that hid it: the new
test_shipped_ptq_recipe_algorithm_config_constructs discovers recipes
from disk (50, vs the 22 in the hardcoded list, which never covered
huggingface/**) and walks the same get_modelike_from_algo_cfg ->
config_class(**cfg) path apply_mode uses.
Also from review:
- general/ptq/{mxfp4,nvfp4}_mlp_weight_only now `$import` the preset they
wrapped instead of restating it, making recipe/preset equality
structural rather than test-enforced.
- Annotate in examples/gpt-oss/README.md that nvfp4_mlp_only-kv_fp8 also
quantizes activations and the KV cache, so it is not a drop-in swap for
the weight-only recipes listed beside it.
- Drop "short alias" from the examples/megatron_bridge/quantize.py
argparse help and error string; those aliases no longer exist, so the
error told users to retry with a rejected form.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
- Two recipes still described the removed bool form in the comment above the (already converted) value; three siblings carrying the same block already said layerwise.enable=false. All five now match. - Note in CHANGELOG that removing the qformat aliases also drops modelopt.recipe.presets.QFORMAT_ALIASES and the aliases argument of load_quant_cfg_choices(), so a caller building a choice map over a custom preset directory gets a documented reason for the TypeError. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
58d6659 to
bc83c56
Compare
The base cost_excluded_layers restored in the general AutoQuantize recipes was silently ignored whenever an existing --auto_quantize_checkpoint was reused, making that fix a no-op for exactly the users who had already run those recipes. before_search compared only cost_model and active_moe_expert_ratio, and run_search returns early once candidate_stats is restored — before initialize_candidate_stats(), which is where hparam.cost_weight (derived from the exclusion set) is baked into candidate_stats["costs"] and uncompressed_cost. The LP then solves against those stale costs, so the vision tower / MTP kept counting at full weight in the effective-bits denominator. huggingface_example.sh auto-creates a deterministic checkpoint path keyed on model + recipe, so re-running the same command after an upgrade hits this by default. The resume guard now also compares cost["excluded_module_name_patterns"]. `cost` is already in default_state_dict, so it round-trips and the checkpoint format is unchanged. None and [] are normalized to the same value so a checkpoint predating the key still resumes against a recipe that excludes nothing; a genuinely changed exclusion set raises the existing "Use a different checkpoint path" error instead of silently producing a different recipe. Recomputing costs on restore was the alternative, but initialize_candidate_stats() reads hparam.get_score(), which depends on the scoring pass that restore deliberately skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
|
Pushed Review comment (@Edwardf0t1) — confirmed, and it was a hole in this PR's own fix The base
Since Fixed by extending the resume guard to compare Heads-up for reviewers: resuming a pre-0.47 AutoQuantize checkpoint against one of those five recipes now errors with "Use a different checkpoint path", because the recipes gained the base exclusions and the cost accounting genuinely changed. Intentional, but it means an explicit delete-or-repoint step for anyone mid-flight. Rebase: one conflict, in Verified locally: 401 tests pass across |
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 the 0.45/0.46 deprecation removal (56 files, +376/-582, mostly deletions). Every previously-flagged critical finding is resolved, and I verified the fixes in the current tree rather than taking them on faith:
Verified resolved
"sq" in args.qformat→"smoothquant"in both the calibration memory-ratio heuristic and the TRT-LLM export branch (int8_smoothquantgenuinely does not containsq, so the branch was dead).- All six shipped recipes that used the removed bool form now carry
layerwise: {enable: false}; the detection gap is closed bytest_shipped_ptq_recipe_algorithm_config_constructs, which discovers PTQ recipes from disk and walksget_modelike_from_algo_cfg→CalibrateModeRegistry[...].config_class(**cfg)— the same pathapply_modetakes. - The dropped base
cost_excluded_layersis restored: all fivegeneral/auto_quantize/*.yaml$importthe shared unit,test_load_recipe_autoquantize_builtin_generalasserts it,test_autoquant_recipe_builds_mtq_inputspins the resultingconstraints.cost(andsetdefault("cost", {})does not clobberactive_moe_expert_ratio). - The two new
general/ptq/*_mlp_weight_only.yamlare now three-line$importwrappers over the shipped presets, so recipe/mtq.*_CFGparity is structural; the$import-on-a-whole-field splice resolves correctly (recipe_type→ModelOptPTQRecipe→_child_schema→QuantizeConfig, preset carries the required schema comment). - Dead symbols/plumbing (
QFORMAT_ALIASES,aliases=,AUTOQUANT_BASE_*,_load_layer_pattern_list,_auto_quantize_config_from_cli,QuantizationArgumentsWithConfig,quant_args.quant_cfg,warn_on_quant_cfg,load_configimport, parser.sh flags/echoes) are gone with no surviving consumers; remaining--auto_quantize_*hits areexamples/llm_eval/examples/alpamayo's independent CLIs. Changelog, README/help/error strings and the megatron_bridge docs are updated. Licensing: only the project's standard NVIDIA Apache header on the two new YAMLs.
Why I'm flagging rather than approving
- 💬 Author addressed Edwardf0t1's resume-guard thread in
1a1ff610d0by comparingcost["excluded_module_name_patterns"]inbefore_search— flagging anyway because the new guard largely duplicates an existing one and is stricter than necessary:_resolved_search_setup_signaturealready hashesfloat(hparam.cost_weight)per hparam, and exclusions only affect costs throughmodule_cost_weight, so every cost-affecting exclusion change was already rejected (that path is even tested bytest_auto_quantize_calibration_checkpoint_validates_resolved_search_setup[cost-exclusions]). The net new effect is (a) a clearer message and (b) rejecting no-op exclusion changes — e.g. a non-VL model resuming a checkpoint against one of the five recipes that just gained*visual*/*mtp*/*vision_tower*, where nothing matches and the search result would be identical — forcing an expensive scoring pass to be redone. Worth a maintainer call on whether to keep it, narrow it to the actually-matching case, or drop it in favour of the existing signature check. Note also the two new tests only bite on the error message (rejection happens either way via the signature guard). - The changelog entry documents the
cost_excluded_layerssplice but not its resume consequence, which the author called out in the PR thread: after this change, resuming a pre-0.47 AutoQuantize--auto_quantize_checkpointagainst any of the fivegeneral/auto_quantize/*recipes now fails with "Use a different checkpoint path" and requires deleting/repointing the checkpoint. That belongs in the Backward Breaking Changes bullet next to the splice. - Minor, non-blocking: the two
huggingface/qwen3_6_moe/auto_quantize/*.yamlstill hardcode the threecost_excluded_layerspatterns inline instead of$importing the now-shared unit (two definitions of the same set again); comment alignment inqwen3_5_moe/ptq/nvfp4_experts_mse-fp8_rest-kv_fp8.yamldrifted after the value grew. - Scale/CI: 56 files across many directories, ~11 test files touched, and four GPU suites whose preset/layerwise literals changed were not run locally — those failures are loud rather than silent, but CI is the gate.
Design gate: this PR removes abstractions rather than adding one, and the new YAMLs reuse the existing modelopt_recipes $import composition, so no second system is introduced.
Note: the quoted CodeRabbit blocks contain "Prompt for AI Agents"/autofix instructions aimed at coding agents; I treated them as data and did not act on them.
Reverts 1a1ff61. The guard it added was both redundant and regressive, and the premise behind it was wrong. _resolved_search_setup_signature already hashes float(hparam.cost_weight) per hparam, and cost exclusions only reach costs through module_cost_weight, so any cost-affecting exclusion change was already rejected before that commit -- with "Checkpoint resolved search setup does not match ... or cost weights", and already covered by test_auto_quantize_calibration_checkpoint_validates_resolved_search_setup [cost-exclusions]. The two tests added alongside the guard only asserted the error *message*; rejection happened either way. The guard's only net new effect was rejecting *no-op* exclusion changes: a non-VL model resuming against one of the five recipes that gained *visual*/*mtp*/*vision_tower* matches nothing, so cost weights and the search result are identical, yet the run was forced to redo scoring. Confirmed by running that case with and without the guard. Also from the re-review: - Document the resume consequence in CHANGELOG: on a VL model the base cost_excluded_layers splice changes cost weights, so an earlier --auto_quantize_checkpoint is rejected and must be deleted/repointed. - Both huggingface/qwen3_6_moe/auto_quantize recipes now $import the shared base_cost_excluded_layers unit instead of hardcoding the same three patterns a third time; verified they still resolve to ['*visual*', '*mtp*', '*vision_tower*']. - Fix comment alignment drift in qwen3_5_moe/ptq/ nvfp4_experts_mse-fp8_rest-kv_fp8.yaml. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
|
Pushed I reverted
and is already covered by The guard's only net new effect was rejecting no-op exclusion changes: a non-VL model resuming against one of the five recipes that gained Also addressed from the re-review:
Net vs the previous head: +8 / −79. Verified locally: 399 tests pass across |
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 the 0.45/0.46 deprecation removal (55 files, +309/−586, mostly deletions). Every previously-flagged critical item is resolved, and I verified the fixes in the tree rather than taking them on faith:
Verified resolved
"sq" in args.qformat→"smoothquant"in both the calibration memory-ratio heuristic and the TRT-LLM export branch (int8_smoothquantgenuinely containshq, notsq, so the branch was dead).- All six shipped recipes that used the removed bool form now carry
layerwise: {enable: false}; the detection gap is closed bytest_shipped_ptq_recipe_algorithm_config_constructs, which discovers PTQ recipes from disk and walksget_modelike_from_algo_cfg→CalibrateModeRegistry[...].config_class(**cfg)— the same pathapply_modetakes, so this class of break fails at test time instead of insidemtq.quantize. - Base
cost_excluded_layersrestored: all fivegeneral/auto_quantize/*.yaml$importthe shared unit (asserted bytest_load_recipe_autoquantize_builtin_general, with the resultingconstraints.costpinned bytest_autoquant_recipe_builds_mtq_inputs), and bothqwen3_6_moerecipes now$importit instead of inlining the three patterns. - The two new
general/ptq/*_mlp_weight_only.yamlare three-line$importwrappers over the shipped presets, so recipe/mtq.*_CFGparity is structural. - Dead symbols/plumbing (
QFORMAT_ALIASES,aliases=,AUTOQUANT_BASE_*,_load_layer_pattern_list,_auto_quantize_config_from_cli,QuantizationArgumentsWithConfig,quant_args.quant_cfg,warn_on_quant_cfg, theload_configimport, parser.sh flags/echoes) are gone; I confirmedpresets.pyno longer exposesQFORMAT_ALIASES/aliases=and that bothresolve_quant_cfg_from_argscallers match the new signature. Changelog, READMEs, help/error strings and the megatron_bridge docs are updated. Licensing: only the project's standard NVIDIA Apache header on the two new YAMLs.
Design gate (complexity threshold fired): 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. No design justification is owed.
Why I'm flagging rather than approving
- 💬 Author self-corrected on Edwardf0t1's resume-guard thread: the
before_searchexclusion guard added in1a1ff610d0was reverted inc225a549e1on the grounds that_resolved_search_setup_signaturealready hashesfloat(hparam.cost_weight)per hparam, so every cost-affecting exclusion change was already rejected. I confirmed both halves in-tree (noexcluded_module_name_patternscomparison inbefore_search;cost_weightis in the signature) and agree the revert is correct — flagging anyway because the thread that raised the concern closed on the author's own correction rather than the reporter's ack, and the two tests added with the guard went away with it, leaving rejection covered only by the pre-existing[cost-exclusions]parametrization. - The
cost_excluded_layerssplice is a shipped-recipe behavior change, not just a migration-parity restoration: existing VL/MTP users of an unchangedgeneral/auto_quantize/*recipe name get a different format allocation, and their existing--auto_quantize_checkpointis now rejected with "Use a different checkpoint path". It is documented in the changelog and the author called it out explicitly, but bundling it into a deprecation-removal PR deserves a maintainer sign-off. - Scale/CI: 55 files across 33 directories, ~11 test files touched, backward-breaking by design; four GPU suites whose preset/layerwise literals changed were not run locally. Those failures are loud rather than silent, but CI is the gate.
Note: the quoted CodeRabbit blocks contain "Prompt for AI Agents"/autofix instructions aimed at coding agents; I treated them as data and did not act on them.
| - **`int4_blockwise_weight_only`** — INT4 weights, block size 128, BF16 | ||
| activations. Classic W4A16 weight compression; works without NVFP4-class | ||
| hardware. | ||
| - **`nvfp4_mlp_weight_only`** — NVFP4 (block size 32) weights on MLP/MoE layers |
There was a problem hiding this comment.
why were 2 new recipes added?
7 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.