Skip to content

test: direct unit suite for the QuantModule machinery (35 tests)#1925

Closed
arham766 wants to merge 1 commit into
NVIDIA:mainfrom
arham766:tests/quant-module
Closed

test: direct unit suite for the QuantModule machinery (35 tests)#1925
arham766 wants to merge 1 commit into
NVIDIA:mainfrom
arham766:tests/quant-module

Conversation

@arham766

@arham766 arham766 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Type of change: new tests

Part of the unit-coverage initiative in #1902. Direct unit suite for modelopt/torch/quantization/nn/modules/quant_module.py — the module-conversion machinery exercised indirectly everywhere but never directly (non-overlap map vs test_forward_patching / test_module_registry / test_quantize_replace / test_quant_linear verified by review). Covers registry class-swap + dynamic-class caching, _setup quantizer wiring and default descriptors, forward wrapping with exact-value equality, the external monkey-patched-forward branch (receives quantized input; export restores the patch — covered nowhere else), quantize_weight/fold_weight semantics incl. exception-safety, modelopt_post_restore device handling, ParallelState, export class restoration. Adversarial review: 3/3 mutations killed. Genuine crash documented and independently reproduced: fold_weight derives the weight attr as name[:-10] (strips '_quantizer', not the '_weight_quantizer' its comment claims), so custom per-weight quantizers from quantizer_attr_names() trip the hasattr assertion — and neither _QuantLlama4TextExperts nor _QuantGptOssExperts overrides fold_weight, so public mtq.fold_weight() on a quantized Llama-4/GPT-OSS model crashes. Any fix must also respect those weights' transposed quantization; happy to send it as a follow-up.

Usage

N/A — tests only.

Testing

Hermetic, CPU-only, deterministic, <1s. Full tests/unit/torch/quantization dir green alongside (789 passed, pre-existing skips only). Adversarially reviewed with independent re-derivation and mutation testing as described.

Before your PR is "Ready for review"

  • Is this change backward compatible?: ✅
  • 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

Targets the module-conversion plumbing itself, complementing the large
indirect surface: registry class-swap with dynamic-class caching,
_setup quantizer wiring and default descriptors, forward wrapping with
exact-value equality vs manual quantizer application, the external
monkey-patched-forward branch (quantized input + export restore -
covered nowhere else), quantize_weight/fold_weight semantics,
modelopt_post_restore device handling, ParallelState, and export class
restoration. Adversarially reviewed: 3/3 seeded mutations killed.

Documents a genuine crash: fold_weight derives the weight name with
name[:-10], stripping '_quantizer' instead of '_weight_quantizer' as
its comment claims - custom per-weight quantizers
(quantizer_attr_names('gate_up_proj')) trip the hasattr assertion, and
neither _QuantLlama4TextExperts nor _QuantGptOssExperts overrides
fold_weight, so public mtq.fold_weight() on a quantized Llama-4 or
GPT-OSS model crashes (reproduced; any fix must also respect the
transposed quantization of those weights).

Part of the coverage initiative in NVIDIA#1902.

Signed-off-by: arham766 <arhamislam766@yahoo.com>
@arham766 arham766 requested a review from a team as a code owner July 6, 2026 02:46
@copy-pr-bot

copy-pr-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown

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

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@arham766, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d8d36652-0502-467e-8716-3ff5c941b530

📥 Commits

Reviewing files that changed from the base of the PR and between b96a785 and 72b3907.

📒 Files selected for processing (1)
  • tests/unit/torch/quantization/test_quant_module.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@arham766

arham766 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Consolidated into #1927 per maintainer feedback in #1902 — the suite was trimmed to only the lines codecov reports uncovered, with parametrization clusters deduplicated. Closing in favor of that PR.

@arham766 arham766 closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant