Repository navigation
fix(torchlib): clamp quantize_per_tensor to quant_min/quant_max - #3052
Open
Mohammed Alkindi (MohammedAlkindi) wants to merge 2 commits into
Open
Mohammed Alkindi (MohammedAlkindi) wants to merge 2 commits into
Mohammed Alkindi (MohammedAlkindi) wants to merge 2 commits into
Conversation
quantize_per_tensor accepted quant_min and quant_max but emitted only QuantizeLinear, which saturates to the full range of dtype. quantize_per_channel already clips to the explicit bounds for this reason; this applies the same treatment to the per-tensor path.
Copilot started reviewing on behalf of
G. Ramalingam (gramalingam)
October 2, 2026 04:08
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Tensor-valued parameters in the registered .tensor2 overload are incorrectly converted into constants.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds explicit range clamping to per-tensor quantization to match PyTorch semantics.
Changes:
- Clips quantized values to
quant_min/quant_max. - Adds an end-to-end reduced-range regression test.
| File | Description |
|---|---|
quantized_decomposed.py |
Adds per-tensor range clipping. |
e2e_ops_tests.py |
Tests reduced-range quantization. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+41
to
+48
| quantized = op.QuantizeLinear(input, scale, common.constant(zero_point, dtype=dtype)) | ||
| # QuantizeLinear saturates to the full range of ``dtype``. PyTorch clamps to the | ||
| # explicit ``quant_min``/``quant_max`` instead, so clamp to match its semantics. | ||
| return op.Clip( | ||
| quantized, | ||
| common.constant(quant_min, dtype=dtype), | ||
| common.constant(quant_max, dtype=dtype), | ||
| ) |
Collaborator
|
Please address copilot comments |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3052 +/- ##
==========================================
- Coverage 72.73% 72.73% -0.01%
==========================================
Files 265 265
Lines 32358 32359 +1
Branches 3069 3069
==========================================
Hits 23536 23536
- Misses 7779 7780 +1
Partials 1043 1043 ☔ View full report in Codecov by Harness. |
The .tensor and .tensor2 overloads pass zero_point, quant_min and quant_max as tensors, so they reach the function as graph values. common.constant cannot materialize a graph value into an initializer, and exporting .tensor2 failed with "int() argument must be a string, a bytes-like object or a real number, not 'SymbolicTensor'". Cast a graph value in the graph instead, and keep the constant path for the scalar overload.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
quantized_decomposed_quantize_per_tensortakesquant_minandquant_maxand never uses them. It emits onlyQuantizeLinear, which saturates to the full range ofdtype, so a reduced range is silently ignored.Its sibling
quantized_decomposed_quantize_per_channelin the same file already handles this, and its comment says why: "QuantizeLinear saturates to the full range ofdtype. PyTorch clamps to the explicitquant_min/quant_maxinstead." The per-tensor path never got the same treatment.Measured in onnxruntime on opset 20, scale 1.0, zero_point 0,
quant_min=0,quant_max=20, int8:The second row is what ships today; the third matches PyTorch's reference,
clamp(round(input/scale) + zero_point, quant_min, quant_max).The test mirrors
test_quantize_per_channel_clamps_to_quant_min_maxdirectly above it.ruff checkandruff format --checkon ruff 0.15.1 with the repo'spyproject.tomlboth pass on the two changed files.Could not verify: I did not run the nox matrix across the torch-nightly, onnx-weekly and ort-nightly variants. The behavioural evidence above comes from onnxruntime directly rather than through a torch export.