fix: reject amax assignment on dynamic quantizers

[arham766]

What does this PR do?

Type of change: Bug fix

Overview: Setting amax on a dynamic TensorQuantizer silently stored a value dynamic quantization never uses; the setter now fails fast. getattr(self, "_dynamic", False) rather than self._dynamic because __init__ assigns self.amax = amax before set_from_attribute_config creates the _dynamic attribute — a bare attribute access would break the amax= constructor argument (all 28 assignment sites audited).

Scope note: this PR originally also fixed the extra_repr dead code in the disabled branch (built a detail string, returned the literal "disabled"). #1879 has since made that branch reachable on main, so that part is now upstream; this PR keeps a regression test pinning the disabled extra_repr detail output and retains only the amax-setter fix. Rebased accordingly.

Testing

tests/unit/torch/quantization/test_tensor_quantizer_cpu.py: test_dynamic_amax_set_rejected (fails without the fix) and test_disabled_extra_repr (pins the now-live disabled branch). Full file: 28 passed.

Before your PR is "Ready for review"

• Is this change backward compatible?: ✅ (only rejects a write that was previously a silent no-op footgun)
• If you copied code from any other sources or added a new PIP dependency, did you follow guidance in CONTRIBUTING.md: N/A
• Did you write any new necessary tests?: ✅
• Did you update Changelog?: N/A
• Did you get Claude approval on this PR?: N/A (external contributor)

Additional Information

Issue: #1902

Summary by CodeRabbit

• Bug Fixes
  • Prevented setting a fixed amax when quantization is configured as dynamic, raising a clear assertion error instead.
  • Improved quantizer status display for disabled quantizers so any additional pre-quantization scale details are preserved in the output.
• Tests
  • Added unit tests covering the disabled quantizer extra_repr() output (including pre-quant scale) and verifying that amax updates are rejected in dynamic mode.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

📝 Walkthrough

Walkthrough

TensorQuantizer now blocks amax assignment in dynamic mode, and unit tests cover that rejection plus disabled extra_repr() output with pre_quant_scale.

Changes

TensorQuantizer Behavior Fixes

Layer / File(s)	Summary
Core TensorQuantizer changes modelopt/torch/quantization/nn/modules/tensor_quantizer.py	TensorQuantizer.amax now rejects assignment when dynamic quantization is enabled, using a guarded _dynamic lookup during initialization.
Unit tests for amax and extra_repr behavior tests/unit/torch/quantization/test_tensor_quantizer_cpu.py	Adds imports and two tests: one checks disabled extra_repr() output with pre_quant_scale, and one checks that setting amax on a dynamic quantizer raises an AssertionError matching "Dynamic quantization".

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Security Anti-Patterns	✅ Passed	Changed modelopt/test Python files contain no banned deserialization, remote-code, eval/exec, nosec, or dependency changes.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the main change: rejecting amax assignment on dynamic quantizers.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

modelopt/torch/quantization/nn/modules/tensor_quantizer.py (1)

345-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the getattr ordering dependency.

The fix is correct — getattr(self, "_dynamic", False) is necessary because __init__ calls self.amax = amax (line 201-202) before _dynamic is set by set_from_attribute_config (line 205). This is exactly the kind of non-obvious constraint that should be captured with a short inline comment, otherwise a future refactor could replace it with self._dynamic (matching the getter at line 342) and reintroduce an AttributeError on TensorQuantizer(amax=...) construction.

As per coding guidelines, "Comment cautiously... Use comments only for non-obvious intent or constraints that remain unclear from the code."

💡 Suggested inline comment

     `@amax.setter`
     def amax(self, value):
         assert value is not None, "amax cannot be set to None."
+        # _dynamic is not yet set when __init__ assigns `amax` before calling
+        # set_from_attribute_config, so default to False via getattr here.
         assert not getattr(self, "_dynamic", False), (
             "Dynamic quantization does not have fixed amax; amax cannot be set."
         )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modelopt/torch/quantization/nn/modules/tensor_quantizer.py` around lines 345
- 355, Add a short inline comment in TensorQuantizer.amax setter near the
getattr(self, "_dynamic", False) check to document the initialization-order
dependency: __init__ sets amax before _dynamic is assigned, so using getattr
avoids AttributeError during construction. Keep the comment focused on this
non-obvious constraint and reference the amax setter and _dynamic initialization
path so future refactors don’t replace it with a direct self._dynamic access.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@modelopt/torch/quantization/nn/modules/tensor_quantizer.py`:
- Around line 345-355: Add a short inline comment in TensorQuantizer.amax setter
near the getattr(self, "_dynamic", False) check to document the
initialization-order dependency: __init__ sets amax before _dynamic is assigned,
so using getattr avoids AttributeError during construction. Keep the comment
focused on this non-obvious constraint and reference the amax setter and
_dynamic initialization path so future refactors don’t replace it with a direct
self._dynamic access.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 45413200-1064-4069-9a60-9ff6af558626

📥 Commits

Reviewing files that changed from the base of the PR and between b96a785 and 6a5fb4e.

📒 Files selected for processing (2)

• modelopt/torch/quantization/nn/modules/tensor_quantizer.py
• tests/unit/torch/quantization/test_tensor_quantizer_cpu.py