fix: SequentialQuantizer children no longer match singular wildcards via fused-experts normalization#1911
Conversation
…hing The fused-experts name normalization from NVIDIA#1340 collapses weight_quantizer.<N> to weight_quantizer — intended for per-expert nn.ModuleList layouts, but this is exactly the child naming of a SequentialQuantizer stored at attribute weight_quantizer. Its children are TensorQuantizers, so they matched singular wildcards like *weight_quantizer on their own, with two user-visible consequences: - applying the same list config twice via set_quantizer_attributes_full nested a SequentialQuantizer inside each sub-slot (non-idempotent, corrupt state no consumer supports); - set_quantizer_attributes_partial with a list config and a wildcard raised ValueError mid-iteration for targets that are already SequentialQuantizers, contradicting its own docstring and leaving the model partially configured. Skip the normalized match when the quantizer's direct parent is a SequentialQuantizer. Raw wildcard matches (including explicit *weight_quantizer.0), callable filters, and the plural weight_quantizers.N fused-experts path are unchanged; the parent lookup only runs when the raw match already failed. Part of the findings in NVIDIA#1902. Signed-off-by: arham766 <arhamislam766@yahoo.com>
|
Warning Review limit reached
Next review available in: 29 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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
1 participant
What does this PR do?
Type of change: Bug fix
Bug 1 in #1902 (regression from #1340): the fused-experts name normalization collapses weight_quantizer.N — exactly a SequentialQuantizer's child names — so sub-quantizers matched *weight_quantizer on their own. Consequences reproduced: re-applying a list config nests SequentialQuantizers inside sub-slots (corrupt state), and set_quantizer_attributes_partial with a list + wildcard raises mid-iteration against its own docstring. Fix skips the normalized match only when the direct parent is a SequentialQuantizer; raw wildcards, callable filters, and the plural weight_quantizers.N path are unchanged, and all #1340 fused-experts tests still pass. Two regression tests pin idempotent re-application and the docstring's list-on-sequential contract.
Usage
N/A
Testing
Regression tests included (added to existing unit test files; each was verified to fail with the fix reverted). Full tests/unit/torch quantization+export+utils+opt battery passes locally with all sibling fixes applied (962 passed). Note: the test-suite PRs #1903-#1907 contain behavior-documenting NOTE tests that pin the OLD behavior fixed here — whichever lands second will be rebased to flip those assertions (happy to do so).
Before your PR is "Ready for review"
Additional Information
Issue: #1902