Skip to content

Fix ModelOpt integration for the >=0.44 list quant_cfg format - #14536

Open
shengliangxu wants to merge 1 commit into
huggingface:mainfrom
shengliangxu:shengliangx/fix-modelopt-quant-cfg
Open

Fix ModelOpt integration for the >=0.44 list quant_cfg format#14536
shengliangxu wants to merge 1 commit into
huggingface:mainfrom
shengliangxu:shengliangx/fix-modelopt-quant-cfg

Conversation

@shengliangxu

Copy link
Copy Markdown

What does this PR do?

Fixes NVIDIA/Model-Optimizer#2001

ModelOpt 0.44 changed quant_cfg and mtq.config._default_disabled_quantizer_cfg from a {pattern: cfg} mapping to a list of {"quantizer_name": ...} entries. NVIDIAModelOptConfig.get_config_from_quant_type spread the default-disabled set into a dict literal, so on ModelOpt >=0.44 building the config raised TypeError: 'list' object is not a mapping — hitting the default NVIDIAModelOptConfig(quant_type=...) path.

Emit quant_cfg in whichever shape the installed ModelOpt expects, keyed off the shape ModelOpt exposes for its default-disabled set: a mapping for <0.44 (unchanged behavior) and the native list of entries for >=0.44 (no deprecation warning). The modules_to_not_convert loop in the quantizer appends a list entry or assigns a mapping key accordingly.

Add a CPU-only regression test that builds the config across quant types and checks the installed ModelOpt accepts it — the existing GPU tests are nightly/big-accelerator gated, so this config-construction crash slipped through CI.

Self-review — Fix ModelOpt integration for the >=0.44 list quant_cfg format

Scope: src/diffusers/quantizers/quantization_config.py, src/diffusers/quantizers/modelopt/modelopt_quantizer.py, tests/quantization/modelopt/{__init__.py,test_modelopt.py}.

Blocking issues

None. The fix is correct on both supported ModelOpt eras — verified by running the new test against real 0.43.0 (dict branch) and 0.45.0 (list branch), 6/6 each. The <0.44 dict path is byte-for-byte identical to the pre-fix code (checked against the original method extracted from git).

Non-blocking issues

  1. One-caller test helper — inline candidate. _weight_quantizer_num_bits (tests/quantization/modelopt/test_modelopt.py:46) has a single caller. Per .ai/AGENTS.md: "If a private helper has only one caller, inlining it at the call site is usually the cleaner choice." It names a real dict-vs-list abstraction, so it's defensible; by the rule it leans toward inlining. Trivial.
  2. CPU validation via a ModelOpt-internal API (advisory for the maintainer). The test asserts acceptance with mtq.config.QuantizeConfig(**modelopt_config) (test_modelopt.py:88) rather than a full mto.apply_mode on a model — a deliberate CPU-only tradeoff so the regression is caught without a GPU. Worth surfacing to the reviewer, not fixing.

Rule checks that passed (not issues)

  • No defensive code (AGENTS.md): the isinstance(default_disabled_quantizer_cfg, dict) branch and the elif "enable" not in entry: entry["enable"] = True fallback are both reachable and tested (dict path on 0.43; the enable:True case on FP8 weight_only=False), so they're not "just in case."
  • Ephemeral context (review-rules.md §Common mistakes): all three comments state the reason and stand alone; the doc URL is a stable pointer. None are PR-scoped chatter.
  • Testing conventions (testing.md): backend-tier config unit test mirroring the retained TestTorchAoConfig pattern — pytest-style, @is_quantization/@is_modelopt markers (both registered in conftest.py), license header. Pipeline/model tester-mixin rules don't apply. No LoRA/slow/integration added.
  • Copied code: none of the touched code is under a # Copied from header.
  • Style: ruff check + ruff format --check clean.

Dead code (advisory)

Rubric's dead-code pass is scoped to new-model PRs — N/A here.

path:line status reason
quantization_config.py:880 (dict branch) Used exercised on ModelOpt <0.44 (verified on 0.43.0)
quantization_config.py:894 (enable:True) Used FP8 weight_only=False param in the test
modelopt_quantizer.py:173 (list append) Used >=0.44 modules_to_not_convert path

Documentation impact

No public API changed (behavior-restoring fix). docs/source/en/quantization/modelopt.md documents the public quant_type/modules_to_not_convert/… params — untouched, no staleness introduced. No agent-guide addition warranted (the dict->list gotcha is captured in the code comment + doc link).

Summary — READY

Minimal, correct on both ModelOpt eras, lint-clean, and covered by a CPU regression test that would have caught the original TypeError (which the pre-existing GPU/nightly-gated tests could not).

  • Fix before submitting: nothing required.
  • Optional polish (your call): inline _weight_quantizer_num_bits (Add glide modeling files #1).
  • Leave for the actual review: the QuantizeConfig-vs-apply_mode CPU-validation tradeoff (+ cosine schedule and unet config #2) — flag it to the maintainer rather than pre-emptively changing it.

Before submitting

  • [ x ] Did you use an AI agent (Claude Code, Codex, Cursor, etc.) to help with this PR? If so:
    • [ x ] Did you read the Coding with AI agents guide?
    • [ x ] Did you run the self-review skill on the diff?
    • [ x ] Did you share the final self-review notes in the PR description or a comment?
  • [ x ] Did you read the contributor guideline?
  • [ x ] Did you read our philosophy doc? (important for complex PRs)
  • [ x ] Was this discussed/approved via a GitHub issue or the forum? Please add a link to it if that's the case.
  • [ x ] Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • [ x ] Did you write any new necessary tests?
  • [ x ] Are you the author (or part of the team) of the model/pipeline (only applicable for model/pipeline related PRs)?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

ModelOpt 0.44 changed `quant_cfg` and `mtq.config._default_disabled_quantizer_cfg`
from a `{pattern: cfg}` mapping to a list of `{"quantizer_name": ...}` entries.
`NVIDIAModelOptConfig.get_config_from_quant_type` spread the default-disabled set
into a dict literal, so on ModelOpt >=0.44 building the config raised
`TypeError: 'list' object is not a mapping` (NVIDIA/Model-Optimizer#2001) — hitting
the default `NVIDIAModelOptConfig(quant_type=...)` path.

Emit `quant_cfg` in whichever shape the installed ModelOpt expects, keyed off the
shape ModelOpt exposes for its default-disabled set: a mapping for <0.44 (unchanged
behavior) and the native list of entries for >=0.44 (no deprecation warning). The
`modules_to_not_convert` loop in the quantizer appends a list entry or assigns a
mapping key accordingly.

Add a CPU-only regression test that builds the config across quant types and checks
the installed ModelOpt accepts it — the existing GPU tests are nightly/big-accelerator
gated, so this config-construction crash slipped through CI.

Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Public preset quant_cfg changed from dict to list in 0.44 (breaking, undocumented?)

1 participant