Repository navigation
[OMNIML-6228] Shrink the input of the conv/IsInf ONNX test model - #2947
Conversation
build_conv_isinf_model builds a 6x32x900x256 fp32 input (177 MB per activation, and the
Conv/Cast/IsInf/Where chain keeps several of them), but the three tests that use it check node
dtypes and QDQ placement, never values. Measured as the peak summed RSS of the pytest process tree
(imports included) on a CPU host, test_autocast.py peaks at 2.6 GB and test_qdq_rules_int8.py at
2.3 GB; with a 1x32x64x64 input they peak at 1.3 GB and 1.5 GB and run 3.5 s and 1.6 s faster.
The assertions are unchanged.
Under pytest-xdist the memory adds up: four workers run files at once, and each worker that has
used cpu_dist_workers keeps its two-rank pool alive. Of the 251 unit test files run alone, these
two are the 9th and 10th largest; the eight above them (2.9-3.6 GB) are six files that use the
pool, test_gptq.py (it spawns its own processes) and test_distill.py. A transient spike of 0.8-1.2 GB
in one worker is what tips a 4-vCPU runner over: the hfep PRs' torch_28 unit jobs were stopped by
the runner ("received a shutdown signal") with these tests in flight in 9 of 33 runs.
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 | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. | 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 main change: reducing the input size of the conv/IsInf ONNX test model. 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 tests/_test_utils/onnx/lib_test_models.py, outside themodeloptpackage andexamplesPython paths covered by this check. The diff only changes ONNX test tenso…Full details: Security Anti-Patterns
-
Explanation
PASS. The pull request changes only
tests/_test_utils/onnx/lib_test_models.py, outside themodeloptpackage andexamplesPython paths covered by this check. The diff only changes ONNX test tensor shapes and adds a comment. It adds notorch.load,numpy.load,trust_remote_code,eval,exec,# nosec, or dependency changes.
-
✨ Finishing Touches: 💡 1
-
📝 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 #2947 +/- ##
==========================================
+ Coverage 70.08% 78.18% +8.10%
==========================================
Files 659 659
Lines 74089 77529 +3440
==========================================
+ Hits 51923 60619 +8696
+ Misses 22166 16910 -5256
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:
|
meenchen
left a comment
meenchen
left a comment
There was a problem hiding this comment.
Bot review (gpt-6.1-sol) — DM the bot to share feedback.
LGTM: the smaller fixture preserves graph topology and all dtype/QDQ assertions; human sign-off is required for the justified existing-test fixture edit.
Needs action:
- Confirm the fixture-size reduction in
tests/_test_utils/onnx/lib_test_models.py; all three callers check structure and precision, not large-shape behavior.
|
What does this PR do?
Type of change: Bug fix (tests)
build_conv_isinf_modelbuilds a 6x32x900x256 fp32 input (177 MB per activation, and the Conv/Cast/IsInf/Where chain keeps several). The three tests that use it (test_conv_isinf_conversion[13|21]intests/unit/onnx/autocast/test_autocast.py,test_conv_isinf_int8intests/unit/onnx/quantization/test_qdq_rules_int8.py) check node dtypes and QDQ placement, never values. The input is now 1x32x64x64; the assertions are unchanged.Measured as the peak summed RSS of the pytest process tree (imports included) on a CPU host, run per file:
test_autocast.pytest_qdq_rules_int8.pyWhy it matters: since the unit tests run under pytest-xdist (#2845), four workers run files at once on a 4-vCPU runner, and each worker that has used
cpu_dist_workerskeeps a two-rank pool alive. Across the 251 unit files run alone, these two are the 9th and 10th largest (the eight above them are six pool-based quantization/export files,test_gptq.pyandtest_distill.py). Themulti-version (unit-3.12(torch_28, ...))job has been stopped by the runner ("The runner has received a shutdown signal", no failed test) 22 times since xdist merged: 15% of its 146 runs, 4 of 65 on branches other than the hfep stack, and no other torch version. It was reported on hfep #2752 five attempts in a row. These stops come from the whole job's memory (the CI speed-up work measured 14,608-15,708 MiB at the peak with--dist loadfileand is working on the structural part, the pool kept per worker), so this PR removes one 0.8-1.2 GiB spike, not the whole cause.Usage
N/A
Testing
tests/unit/onnx/autocast/test_autocast.pyandtests/unit/onnx/quantization/test_qdq_rules_int8.py: 31 passed on current main with the change applied.Before your PR is "Ready for review"
CONTRIBUTING.md: N/ASummary by CodeRabbit