Repository navigation
Conversation
weight.reshape(block_size, -1) on a (ci, co) weight does not slice ci into contiguous blocks of block_size rows: it reinterprets the flat buffer, so each reshaped row draws from every block_size-th original row instead of block_size consecutive ones. The per-block max used as the AWQ-Lite search scale ends up computed against the wrong rows. Reshape to (n_blocks, block_size, co) instead, which keeps each block's rows together without needing a transpose, take the max over the block axis, then reshape back. Confirmed correct against a naive per-block reference for the divisible, padded, and no-block-size code paths, and confirmed the new regression tests fail against the old implementation. Signed-off-by: Siddhardha Nanda <99672439+SID-6921@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the primary fix: preventing ONNX AWQ-Lite get_weight_scale from mixing rows across blocks. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Security Anti-Patterns PASS. The PR changes only modelopt/onnx/quantization/int4.pyamong the package/examples Python files, plus tests andCHANGELOG.rst. The added code performs NumPy reshape, max, division, and commen…Full details: Security Anti-Patterns
-
Explanation
PASS. The PR changes only
modelopt/onnx/quantization/int4.pyamong the package/examples Python files, plus tests andCHANGELOG.rst. The added code performs NumPy reshape, max, division, and comments only. It adds notorch.load(..., weights_only=False),numpy.load(..., allow_pickle=True), hardcodedtrust_remote_code=True, external-inputeval()/exec(), or# nosec. Nopyproject.tomlorrequirements.txtfile changed, so no new PIP dependency was introduced.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
get_weight_scaleinmodelopt/onnx/quantization/int4.pyreshapes the weight directly into(block_size, -1)to compute a per-block max. For a weight shaped(ci, co), this does not sliceciinto contiguous blocks ofblock_sizerows — it reinterprets the flat buffer, so each reshaped row draws from everyblock_size-th original row instead ofblock_sizeconsecutive ones.Concretely, for an 8x1 weight with
block_size=4where rows 0 and 1 (both in the same block) hold 10 and 5:This feeds
AWQLiteHelper.weight_scale, whichget_scale()uses to pick the AWQ-Lite search scale during ONNX INT4 PTQ — the default block size for ONNX INT4 quantization is 128, so this is live on the default code path and silently skews the search scale with no crash or warning.Fix: reshape to
(n_blocks, block_size, co)instead, which keeps each block's rows together without needing a transpose (a pure reshape of the leading axis, no copy), take the max over the block axis, then reshape back. The no-block-size and padded-input code paths are unaffected.Added
tests/unit/onnx/quantization/test_int4.pywith a minimal reproduction of the original bug plus cross-checks against a naive per-block reference for the divisible, padded, and no-block-size cases. Confirmed the new tests fail against the original implementation (mutation-tested) and pass against the fix.Note: I verified this with standalone scripts rather than
pytestdirectly — this environment'stests/unit/onnx/conftest.pyhas an autouse fixture that force-builds a C++ extension requiring MSVC, unavailable here; the same block applies to unrelated, untouched test files, confirming it's an environment limitation and not something this change introduces.Summary by CodeRabbit