[Rishi Dave (Rishi-Dave)]

Description

Adds a new ActivationRestrictedAsymmetric extra-option to the Python
quantization tools. When enabled, uint8 activation zero-points are snapped
to either 0 (when rmin >= 0, e.g. post-ReLU/Sigmoid tensors) or 128
(when rmin < 0). The scale is recomputed so the dequantized range still
covers [rmin, rmax] without clipping.

This restricted asymmetric mode is required by some hardware accelerators
that only support these two zero-point values for uint8 quantization,
without requiring the full restriction to symmetric (zero-point = 128 for
all tensors).

Motivation and Context

Fixes #21398.

Existing options cover only fully symmetric (ActivationSymmetric →
zero-point fixed at 128) or unrestricted asymmetric. There was no mode
that picks the closer of {0, 128} per tensor based on its observed range.

Changes

• quant_utils.py: new snap_zero_point_to_uint8(rmin, rmax) helper.
• base_quantizer.py: parse new ActivationRestrictedAsymmetric extra-option.
• onnx_quantizer.py and qdq_quantizer.py: apply snap after
  compute_scale_zp in the activation path. Guarded on
  quant_type == UINT8 and not symmetric. Weight and int8 paths are
  untouched.
• quantize.py: document the new option in the four extra_options
  docstrings.
• test_symmetric_flag.py: new TestRestrictedAsymmetricFlag covering
  three cases (positive range → zp=0, signed range → zp=128, and
  option-disabled regression).

Testing

```
python -m pytest onnxruntime/test/python/quantization/test_symmetric_flag.py -v
```
All 7 tests pass (4 existing + 3 new). `lintrunner` is clean.

[Tianlei Wu (tianleiwu)]

Thanks for the focused change. The new option is consistently wired through the QOperator and QDQ paths, and the basic snap behavior is covered. I found one correctness issue that should be fixed before merge: the restricted asymmetric path recomputes scale after the existing quant-param helper and drops the MinimumRealRange guarantee. I also left a smaller test-discovery note.

[Copilot]

Pull request overview

Adds a new Python quantization extra_options mode (ActivationRestrictedAsymmetric) to support uint8 activation zero-points restricted to {0, 128}, as required by some accelerators.

Changes:

• Add snap_zero_point_to_uint8(rmin, rmax) helper to recompute (zp, scale) with zp snapped to 0 or 128.
• Parse/propagate the new ActivationRestrictedAsymmetric option and apply it in both QOperator and QDQ quantization activation parameter calculation.
• Document the option in quantize.py and add unit tests covering the expected snapping behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
onnxruntime/python/tools/quantization/quant_utils.py	Adds snapping helper for restricted-asymmetric uint8 activations.
onnxruntime/python/tools/quantization/base_quantizer.py	Parses new ActivationRestrictedAsymmetric extra option.
onnxruntime/python/tools/quantization/onnx_quantizer.py	Applies snapping in QOperator activation quant-param calculation.
onnxruntime/python/tools/quantization/qdq_quantizer.py	Applies snapping in QDQ activation quant-param calculation.
onnxruntime/python/tools/quantization/quantize.py	Documents the new extra-option in public docstrings.
onnxruntime/test/python/quantization/test_symmetric_flag.py	Adds tests validating zp snapping behavior for positive/signed activation ranges.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Rishi Dave (Rishi-Dave)]

Thanks for the careful review. Pushed d184733 addressing the points:

• snap_zero_point_to_uint8 now takes qmin, qmax, and min_real_range. The snap pivot is computed as mid = (qmin + qmax + 1) // 2 (still 128 for the default 0/255 path), and the negative-rmin / non-negative-rmin branches use mid - qmin and qmax - mid as denominators. Defaults preserve the prior numerics exactly.
• Degenerate rmax <= rmin no longer returns scale=1.0. It derives a meaningful scale from max(|rmin|, |rmax|) and applies the min_real_range floor when provided.
• Both call sites in onnx_quantizer.py and qdq_quantizer.py now forward qmin, qmax, and self.min_real_range, so per-tensor reduce_range and MinimumRealRange flow through correctly when ActivationRestrictedAsymmetric is enabled.
• Added two tests covering the reduce_range (qmax=127) and min_real_range floor paths.

__main__ block was already at the end of the file; left as-is.

[Tianlei Wu (tianleiwu)]

Follow-up review (round 2)

All high-priority concerns from the previous round (3969e7c) have been addressed:

• snap_zero_point_to_uint8 now accepts qmin, qmax, and min_real_range parameters, properly honoring reduce_range and MinimumRealRange settings.
• Both QOperator and QDQ call sites forward these parameters.
• The test class is correctly placed before the if __name__ guard.
• Degenerate range handling is improved.

All 7 previous threads have been resolved. Two minor suggestions remain (see inline comments), neither blocking.

[Tianlei Wu (tianleiwu)]

Suggestion (non-blocking): if a user sets both ActivationSymmetric=True and ActivationRestrictedAsymmetric=True, the restricted path silently does nothing because symmetric=True fails the not symmetric guard in both quantizers. This is almost certainly a misconfiguration. Consider logging a warning here when both flags are enabled:

if self.is_activation_symmetric and self.is_activation_restricted_asymmetric:
    logger.warning("ActivationSymmetric and ActivationRestrictedAsymmetric are mutually exclusive; "
                   "ActivationRestrictedAsymmetric will be ignored.")

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Rishi Dave (Rishi-Dave)]

Thanks for the review. Pushed b58a58a addressing all three points:

• quant_utils.py: in snap_zero_point_to_uint8, the degenerate rmax <= rmin branch now snaps zp to qmin when rmin >= 0 (matching the normal path) instead of unconditionally returning the midpoint. This restores the contract for all-zero / all-positive constant tensors so they get zp=0, not 128.
• quantize.py: updated all four ActivationRestrictedAsymmetric docstrings to say "midpoint of the quantized range [qmin, qmax]" instead of the hardcoded "128", since under reduce_range=True (qmax=127) the midpoint is 64.
• test_symmetric_flag.py: added test_all_zero_activations_zp_is_qmin covering the rmin==rmax==0 case and asserting zp==0. Existing 9 tests still pass; lintrunner clean.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Rishi Dave (Rishi-Dave)]

Thanks for the catch on the latest review — addressed in 53039cb:

• snap_zero_point_to_uint8 now expands [rmin, rmax] to include zero before applying the min_real_range floor, matching the ordering in compute_scale_zp so the floor behaves identically for all-positive tensors.
• In the degenerate-range branch, the scale denominator is now conditional: full (qmax - qmin) when degenerate_zp == qmin (all-positive path, mirrors the non-degenerate rmin >= 0 scale), and (qmax - qmin) // 2 only when degenerate_zp == mid (symmetric snap).
• test_symmetric_flag.py writes the intermediate ONNX models inside a tempfile.TemporaryDirectory() instead of CWD, in line with the other tests in onnxruntime/test/python/quantization/.

lintrunner -a is clean. PTAL.

[Tianlei Wu (tianleiwu)]

Review round 3 (53039cb)

All high-priority concerns from previous rounds have been addressed:

• snap_zero_point_to_uint8 now correctly accepts qmin, qmax, and min_real_range parameters, matching compute_scale_zp semantics (zero-inclusion before min_real_range expansion).
• The degenerate-range branch correctly uses full range for positive-snap (zp=qmin) and half range for mid-snap, consistent with the non-degenerate paths.
• Tests use tempfile.TemporaryDirectory() and the class is placed before the if __name__ guard.
• Docstrings correctly describe "midpoint of the quantized range" rather than hardcoding 128.

The implementation is well-structured: the snap logic is cleanly factored into a single utility function, both QOperator and QDQ paths are consistently wired, and the guard conditions (quant_type == UINT8 and not symmetric) provide correct defense against accidental application to weights or symmetric activations.

One non-blocking suggestion from round 2 remains open (mutual exclusivity warning when both ActivationSymmetric and ActivationRestrictedAsymmetric are True). It is recommended but not a blocker for merge.

LGTM.

[Tianlei Wu (tianleiwu)]

Approving — all concerns from previous rounds have been resolved. See the COMMENTED review above for full details.