Repository navigation
[OMNIML-6228] Seed the quantizer tests that compare against the reference at exact rounding ties - #2914
Conversation
…rounding ties test_per_channel_scale and test_per_tensor_scale of TensorQuantizerTester, and test_per_tensor_scale of TensorQuantCommon, compare a fake-quantized random input with the test's own reference quantizer. The two round an element that sits exactly on a tie (x / step = N.5) in opposite directions, so the outputs are one quantization step apart, which neither rtol=1e-2 nor the default tolerances absorb. Over 20,000 RNG states the unseeded tests fail 8 times for the per-channel test (0.040%, e.g. seeds 1786, 5343, 8185, 11150) and once for each per-tensor test (0.005%, seed 8276); each failure is exactly one element one step off. A serial run draws the same input every time, but under pytest-xdist the RNG state depends on the scheduling, and one such failure hit a unit job of PR #2852. The other methods of TensorQuantCommon pass all 20,000 states. Seed the three tests with 123, as test_per_channel_scale of TensorQuantCommon already does. Under outer RNG states 0..11999, the seeds above included, all three now pass every time. Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly identifies the quantizer tests and the deterministic seeding needed to handle exact rounding ties. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. |
| 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 pull request changes only two Python test utility files under tests/_test_utils/torch/quantization/. It does not change the modelopt package, examples, pyproject.toml, or `requirements… |
Full details: Security Anti-Patterns
-
Explanation
PASS. The pull request changes only two Python test utility files under
tests/_test_utils/torch/quantization/. It does not change themodeloptpackage, examples,pyproject.toml, orrequirements.txt, so none of the specified security anti-pattern conditions apply.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- 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 review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2914 +/- ##
==========================================
+ Coverage 70.08% 78.82% +8.74%
==========================================
Files 659 659
Lines 74081 74081
==========================================
+ Hits 51917 58392 +6475
+ Misses 22164 15689 -6475
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
2 participants
What does this PR do?
Type of change: Bug fix (tests)
TensorQuantizerTester.test_per_channel_scalefailed once in a unit job of PR #2852 (unit-3.12(torch_214, tf_latest), xdist worker gw3). It fake-quantizes an unseeded random tensor and compares the result with the test's own reference quantizer (quantin_test_utils/torch/quantization/quant_utils.py). The two round an element that sits exactly on a tie (x / step = N.5) in opposite directions, so that element ends up one quantization step apart (0.02-0.03 here), whichrtol=1e-2cannot absorb. Reported by the CI speed-up work; I reproduced it.Over 20,000 RNG states on CPU the unseeded tests fail like this:
TensorQuantizerTester.test_per_channel_scaleTensorQuantizerTester.test_per_tensor_scaleTensorQuantCommon.test_per_tensor_scaleEvery failure is exactly one element one step off (seed 1786:
x / step = -74.500000, reference -2.1077, quantizer -2.0796). The other methods ofTensorQuantCommonpass all 20,000 states, and itstest_per_channel_scalealready callstorch.manual_seed(123).A serial run draws the same input every time, so this stayed hidden until pytest-xdist (#2845) made the RNG state depend on the scheduling. About 14 unit jobs of a PR run execute these tests, so a 0.05% failure rate per execution is roughly a 0.7% chance that a PR run needs a re-run.
The three tests now start with
torch.manual_seed(123), like the sibling test does.Usage
N/A
Testing
tests/unit/torch/quantization/test_tensor_quantizer_cpu.pyandtest_tensor_quant_cpu.py: 83 passed.tests/gpu/torch/quantization/test_tensor_quantizer_cuda.pyandtest_tensor_quant_cuda.py. I did not run those on a GPU; a fixed input could in principle tie on the CUDA kernel, which would show up as a deterministic failure in this PR's GPU CI rather than a random one.Before your PR is "Ready for review"
CONTRIBUTING.md: N/ASummary by CodeRabbit