Skip to content

Conversation

@shantoislamdev
Copy link

@shantoislamdev shantoislamdev commented Jan 30, 2026

What does this PR do?

Type of change: Dead code cleanup, new tests

Overview: Remove confusing dead code in find_scales function and add test coverage.

The commented-out line # z = -np.round(temp).clip(...) in quant_utils.py represented a buggy implementation due to operator precedence. Python interprets -np.round(temp).clip(...) as -(np.round(temp).clip(...)), which incorrectly produces 0 for negative input ranges. The active code correctly performs negation before clipping.

This PR:

  1. Removes the confusing commented-out dead code
  2. Adds a parametrized test to verify the zero-point calculation works correctly for various input ranges

Testing

Added test_find_scales_zero_point which tests:

  • Negative-only range
  • Mixed range (negative to positive)
  • Positive-only range
  • Symmetric around zero

Before your PR is "Ready for review"

  • Make sure you read and follow Contributor guidelines and your commits are signed. ✅
  • Is this change backward compatible?: Yes
  • Did you write any new necessary tests?: Yes
  • Did you add or update any necessary documentation?: No (code cleanup, no API changes)
  • Did you update Changelog?: No (minor cleanup, not a feature/bug fix)

Additional Information

Files changed:

Summary by CodeRabbit

  • Chores

    • Removed obsolete comment from quantization utilities code.
  • Tests

    • Added test coverage for asymmetric quantization zero-point calculations to validate scale values, shape consistency, and zero-point bounds.

✏️ Tip: You can customize this high-level summary in your review settings.

Remove commented-out buggy zero-point calculation.
The commented code had operator precedence issues that incorrectly
produced 0 for all negative inputs. The active implementation is correct.

Signed-off-by: shanroislamdev <shantoislamdev@gmail.com>
Add parametrized test to verify find_scales correctly calculates
zero-point for asymmetric quantization across different input ranges.

Signed-off-by: shanroislamdev <shantoislamdev@gmail.com>
@shantoislamdev shantoislamdev requested a review from a team as a code owner January 30, 2026 09:10
@copy-pr-bot
Copy link

copy-pr-bot bot commented Jan 30, 2026

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
Copy link
Contributor

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

A comment line was removed from the find_scales function in the quantization utility module, and a new test case was added to validate asymmetric quantization zero-point calculations. These changes maintain existing functionality while improving test coverage.

Changes

Cohort / File(s) Summary
Code Cleanup
modelopt/onnx/quantization/quant_utils.py
Removed an unused/outdated comment from the zero-point branch within find_scales. No functional changes to computation logic.
Test Coverage
tests/unit/onnx/test_quant_utils.py
Added new test function test_find_scales_zero_point to validate asymmetric quantization behavior, including positive scales, non-None zero-point, shape consistency, and zero-point bounds verification. Updated imports to include find_scales.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: removing dead code (the commented line) from find_scales and adding test coverage for zero-point calculation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

🧪 Unit Test Generation v2 is now available!

We have significantly improved our unit test generation capabilities.

To enable: Add this to your .coderabbit.yaml configuration:

reviews:
  finishing_touches:
    unit_tests:
      enabled: true

Try it out by using the @coderabbitai generate unit tests command on your code files or under ✨ Finishing Touches on the walkthrough!

Have feedback? Share your thoughts on our Discord thread!


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

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