Skip to content

Commit 77902d5

Browse files
shengliangxukinjalpatel27
authored andcommitted
Validate non-empty cfg when enabling quantizers in quant_cfg (#1192)
### What does this PR do? An entry with enable=True and an empty or invalid cfg (e.g., cfg={}, cfg=[], or cfg=42) would pass normalize_quant_cfg_list but later crash in set_quantizer_by_cfg when QuantizerAttributeConfig(enable=True) was constructed with no actual quantizer attributes. Add early validation in normalize_quant_cfg_list to reject such entries at config time rather than surfacing a confusing Pydantic ValidationError deep in the conversion pipeline. ### Testing new unit tests added <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Stricter validation for quantizer entries: enabled quantizers must have a non-empty dict or non-empty list of non-empty dicts; empty or invalid cfg values now raise a clearer error. Normalization still preserves explicit enable/disable flags. * **Tests** * Added unit tests covering rejection of empty/invalid configs when enabled and acceptance when disabled, and ensuring normalized output preserves disabled state. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
1 parent f1f594c commit 77902d5

File tree

2 files changed

+82
-1
lines changed

2 files changed

+82
-1
lines changed

modelopt/torch/quantization/config.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1560,6 +1560,10 @@ def normalize_quant_cfg_list(v: dict | list) -> list[QuantizerCfgEntry]:
15601560
- An empty entry ``{}``.
15611561
- An entry with only ``quantizer_name`` and no other keys — the only effect would be an
15621562
implicit ``enable=True``, which must be stated explicitly.
1563+
- An entry with ``enable=True`` (explicit or implicit) whose ``cfg`` is not a non-empty
1564+
``dict`` or ``list`` — e.g. ``{"quantizer_name": "*", "cfg": {}}`` or
1565+
``{"quantizer_name": "*", "cfg": 42}``. An enabled quantizer must have a valid
1566+
configuration.
15631567
15641568
**Normalization** — after conversion and validation every entry is put into canonical form:
15651569
@@ -1577,7 +1581,8 @@ def normalize_quant_cfg_list(v: dict | list) -> list[QuantizerCfgEntry]:
15771581
15781582
Raises:
15791583
ValueError: If any entry has only ``quantizer_name`` with neither ``cfg`` nor ``enable``,
1580-
or if the entry format is not recognized.
1584+
if ``enable=True`` with an empty or non-dict/list ``cfg``, or if the entry format
1585+
is not recognized.
15811586
"""
15821587

15831588
def _warn_legacy():
@@ -1662,6 +1667,28 @@ def _dict_to_entry(key: str, value) -> list[QuantizerCfgEntry]:
16621667
"enable=True is not allowed; set it explicitly)."
16631668
)
16641669

1670+
# Validate: when cfg is present and enable=True, cfg must be a non-empty
1671+
# dict or list. An empty cfg would attempt to create a
1672+
# QuantizerAttributeConfig with no actual configuration.
1673+
cfg = entry.get("cfg")
1674+
enable = entry.get("enable", True)
1675+
if enable and cfg is not None:
1676+
if isinstance(cfg, dict):
1677+
is_invalid = len(cfg) == 0
1678+
elif isinstance(cfg, list):
1679+
is_invalid = len(cfg) == 0 or any(
1680+
not isinstance(item, dict) or len(item) == 0 for item in cfg
1681+
)
1682+
else:
1683+
is_invalid = True
1684+
if is_invalid:
1685+
raise ValueError(
1686+
f"Invalid quant_cfg entry: {raw!r} — 'cfg' must be a non-empty dict "
1687+
f"or a non-empty list of non-empty dicts when enabling a quantizer "
1688+
f"(got {type(cfg).__name__}: {cfg!r}). Either provide quantizer "
1689+
"attributes in 'cfg' or remove 'cfg' and set 'enable' explicitly."
1690+
)
1691+
16651692
# Normalize: make enable and cfg always explicit.
16661693
entry.setdefault("enable", True)
16671694
entry.setdefault("cfg", None)

tests/unit/torch/quantization/test_config_validation.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,60 @@ def test_error_on_multi_key_legacy_dict(self):
163163
with pytest.raises(ValueError):
164164
normalize_quant_cfg_list([{"*weight_quantizer": {}, "*input_quantizer": {}}])
165165

166+
def test_error_on_empty_cfg_dict_implicit_enable(self):
167+
"""Entry with cfg={} and implicit enable=True is rejected."""
168+
with pytest.raises(ValueError, match="non-empty dict"):
169+
normalize_quant_cfg_list([{"quantizer_name": "*weight_quantizer", "cfg": {}}])
170+
171+
def test_error_on_empty_cfg_dict_explicit_enable_true(self):
172+
"""Entry with cfg={} and explicit enable=True is rejected."""
173+
with pytest.raises(ValueError, match="non-empty dict"):
174+
normalize_quant_cfg_list(
175+
[{"quantizer_name": "*weight_quantizer", "cfg": {}, "enable": True}]
176+
)
177+
178+
def test_error_on_empty_cfg_list_enable_true(self):
179+
"""Entry with cfg=[] and enable=True is rejected."""
180+
with pytest.raises(ValueError, match="non-empty dict"):
181+
normalize_quant_cfg_list(
182+
[{"quantizer_name": "*weight_quantizer", "cfg": [], "enable": True}]
183+
)
184+
185+
def test_error_on_non_dict_non_list_cfg_enable_true(self):
186+
"""Entry with cfg of invalid type (e.g. int) and enable=True is rejected."""
187+
with pytest.raises(ValueError, match="non-empty dict"):
188+
normalize_quant_cfg_list(
189+
[{"quantizer_name": "*weight_quantizer", "cfg": 42, "enable": True}]
190+
)
191+
192+
def test_error_on_cfg_list_with_empty_dict_enable_true(self):
193+
"""Entry with cfg=[{}] and enable=True is rejected (empty dict element)."""
194+
with pytest.raises(ValueError, match="non-empty dict"):
195+
normalize_quant_cfg_list(
196+
[{"quantizer_name": "*weight_quantizer", "cfg": [{}], "enable": True}]
197+
)
198+
199+
def test_error_on_cfg_list_with_non_dict_element_enable_true(self):
200+
"""Entry with cfg=[42] and enable=True is rejected (non-dict element)."""
201+
with pytest.raises(ValueError, match="non-empty dict"):
202+
normalize_quant_cfg_list(
203+
[{"quantizer_name": "*weight_quantizer", "cfg": [42], "enable": True}]
204+
)
205+
206+
def test_empty_cfg_dict_enable_false_accepted(self):
207+
"""Entry with cfg={} and enable=False is allowed (disable-only entry)."""
208+
result = normalize_quant_cfg_list(
209+
[{"quantizer_name": "*input_quantizer", "cfg": {}, "enable": False}]
210+
)
211+
assert result[0]["enable"] is False
212+
213+
def test_empty_cfg_list_enable_false_accepted(self):
214+
"""Entry with cfg=[] and enable=False is allowed (disable-only entry)."""
215+
result = normalize_quant_cfg_list(
216+
[{"quantizer_name": "*input_quantizer", "cfg": [], "enable": False}]
217+
)
218+
assert result[0]["enable"] is False
219+
166220
def test_new_format_with_list_cfg(self):
167221
"""cfg can be a list of dicts for SequentialQuantizer."""
168222
raw = [

0 commit comments

Comments
 (0)