Conversation
🏷️ CI GuideRuns automatically on every PR:
Extended tests (opt-in via labels):
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a new op_tests/tuning_tests/ test suite intended to validate the tuner stack at multiple levels (static CSV validation, CPU-only unit tests for tuner infrastructure, and GPU-required end-to-end tuning smoke tests).
Changes:
- Added Level 0 CSV integrity tests for tuned/untuned config files.
- Added Level 1 unit tests for
base_tunerutilities and simulatedmp_tunerpolling-loop behavior. - Added Level 2 GPU smoke tests that execute several tuner scripts end-to-end and validate output CSVs, plus a small README for running the suite.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| op_tests/tuning_tests/test_csv_validation.py | Level 0: validates tuned/untuned CSV integrity (duplicates, invalid times, etc.). |
| op_tests/tuning_tests/test_tuner_infra.py | Level 1: CPU-only unit tests for base_tuner helpers (CSV I/O, merge/dedup, calculate, post_process). |
| op_tests/tuning_tests/test_mp_tuner_logic.py | Level 1: simulated tests for mp_tuner polling loop behavior (timeouts, KeyError, AcceleratorError). |
| op_tests/tuning_tests/test_tune_pipeline.py | Level 2: GPU-required end-to-end tuning pipeline smoke tests invoking tuner scripts. |
| op_tests/tuning_tests/init.py | Marks tuning_tests as a package. |
| op_tests/tuning_tests/README.md | Documents the structure/levels and how to run the new tuning tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_topk_not_leak_across_shapes(self): | ||
| """BUG REGRESSION: topk must not leak between shapes. | ||
|
|
||
| If shape A has 0 valid candidates (all fail errRatio), topk should NOT | ||
| be permanently set to 0, causing shape B to also return 0 results. | ||
| """ | ||
| tuner = _StubTuner.get() |
There was a problem hiding this comment.
The new regression tests test_topk_not_leak_across_shapes/test_topk_not_shrink_across_shapes will fail against the current GemmCommonTuner.post_process() implementation: it mutates the topk parameter in-place when a shape has fewer valid candidates (if len(filtered_time) < topk: topk = len(filtered_time)), which then carries over to subsequent shapes. Either include the corresponding fix in aiter/utility/base_tuner.py (use a per-shape k = min(requested_topk, len(filtered_time)) without reassigning topk) or adjust/remove these tests.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Motivation
Technical Details
Test Plan
Test Result
Submission Checklist