Add OnnxDiscrepancyCheck speedup metric with default timing updates#2502
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR enhances the OnnxDiscrepancyCheck pass by adding an inference speedup measurement (ONNX vs PyTorch) and introducing configurable warmup/timing iteration settings, with updated defaults and tests to validate the new behavior.
Changes:
- Added
warmup_iterationsandtiming_iterationsconfig parameters (defaultingtiming_iterationsto5) and implemented speedup measurement with an option to skip whentiming_iterations=0. - Updated session/device setup to target the configured accelerator (with CPU fallback) and run PyTorch on the matched torch device.
- Added unit tests to validate the new default and the skip behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
olive/passes/onnx/discrepancy_check.py |
Adds speedup timing, new config params/defaults, and device-aware session/model handling. |
test/passes/onnx/test_discrepancy_check.py |
Adds tests for the new timing_iterations default and the “skip when 0” behavior. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@copilot test/passes/onnx/test_discrepancy_check.py:105: in test_compare_generation_returns_common_prefix_length |
Fixed in 25c6ab4. Added |
…ter and gathered for a dashboard (#2503) ## Describe your changes The pass OnnxDiscrepancyCheck is used to gather metrics on canary models. They need to be dumped to let the user see them in detail, the user can be a dashboard as well. When `--test` is active, `add_discrepancy_check_pass` now injects `max_mae: 0.1` and `report_output_dir` (set to the run config's `output_dir`) into the discrepancy check pass configuration alongside `reference_model_path`. Both `report_output_dir` and the CLI `save_discrepancy_check_results` helper normalize file paths to their parent directory to avoid writing artifacts under a path that looks like a file. Fixed test regressions in `test_workflow_run_command_with_test_override` and `test_workflow_run_command_with_test_reuses_test_output_dir`: - Set `mock_run.return_value = None` so `save_discrepancy_check_results` exits early at its null-guard check instead of attempting to serialize a `MagicMock` via `json.dumps`. - Updated the `assert_called_once_with` expectation to include `max_mae` and `report_output_dir` in the expected discrepancy check pass config. `OnnxDiscrepancyCheck` now raises a `ValueError` after persisting the report when any configured threshold is exceeded (numeric metric thresholds or generation token sequence minimum), failing the run instead of silently succeeding. This restores the documented "the pass fails if any configured threshold is exceeded" behavior for `--test` runs. Replaced `print` statements in `olive/cli/base.py` with `logger.debug` calls to follow the project's logging conventions. Merged upstream changes from main (`#2502`) that added device-aware ONNX session preparation and inference speedup measurement to `OnnxDiscrepancyCheck`. The merge combined the device/torch_device setup and enhanced `prepare_session` call from main with the `report_dir` normalization and reference model export introduced by this branch. ## Checklist before requesting a review - [ ] Add unit tests for this change. - [x] Make sure all tests can pass. - [ ] Update documents if necessary. - [x] Lint and apply fixes to your code by running `lintrunner -a` - [ ] Is this a user-facing change? If yes, give a description of this change to be included in the release notes. ## (Optional) Issue link --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Describe your changes
Added speedup measurement for
OnnxDiscrepancyCheckand updated behavior based on review feedback:timing_iterationsdefault from10to5.timing_iterationsis set to0, speedup measurement is skipped.deviceattribute forcompare_generationtests.Checklist before requesting a review
lintrunner -a