Repository navigation
Fix OpenVINO tests in CI - #24305
Fix OpenVINO tests in CI#24305suryasidd wants to merge 5 commits into
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24305
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New Failures, 1 Unrelated FailureAs of commit de5b0a6 with merge base cd461b0 ( NEW FAILURES - The following jobs have failed:
BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
This PR needs a
|
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are identified for the OpenVINO runtime and CI failure-reporting changes.
No open issues 🎉
What changed in this PR
This PR addresses OpenVINO operator test failures by adjusting runtime buffer handling and scalar tensor creation, and makes test failures visible to CI.
Changes:
- Removes the explicit release of processed model data during backend initialization.
- Handles rank-zero tensors without calling OpenVINO shape allocation or cleanup.
- Returns a nonzero exit code when the test suite fails.
| File | Description |
|---|---|
backends/openvino/tests/test_runner.py |
Exits with status 1 on test failures so CI detects them. |
backends/openvino/runtime/OpenvinoBackend.cpp |
Adjusts model buffer lifetime and rank-zero tensor handling. |
Review effort: Balanced
| device.assign(buf, len); | ||
| } | ||
| target = &device; | ||
| } else if (std::strcmp(compile_spec.key, "inference_precision") == 0) { |
There was a problem hiding this comment.
The title and description read like a test-only fix, but this also changes the runtime that ships in the wheels. There is a new inference_precision compile spec that users can pass. And 0-d (scalar) tensors now work as delegate inputs and outputs, where before they failed. The test runner change matters too. Before, the OpenVINO job stayed green even when tests failed, so the failures were hidden. Now the job blocks pull requests on the op and model tests. Could you list these in the description and add the "release notes: openvino" label?
| device.c_str(), | ||
| "INFERENCE_PRECISION_HINT", | ||
| inference_precision.c_str()); | ||
| if (status != OV_STATUS_OK) { |
There was a problem hiding this comment.
OpenVINO does not check the value in ov_core_set_property. On a new core it only stores it, and checks it later, when the import loads the device plugin. So a typo such as fp32 never reaches this branch. With OpenVINO 2026.4.1 the import fails, init returns Internal, and OpenVINO's own message, which lists the supported values, is not logged. With 2025.1 the import succeeds and the value is quietly read as dynamic. The branch does fire for a device name OpenVINO rejects, but then the log blames the precision. Could we check the value up front (f32, f16, bf16) and return InvalidArgument, the way execute checks its arguments? Logging OpenVINO's error text when the import fails would also help.
| } | ||
| } | ||
|
|
||
| // The device picks its default precision when the model is imported, e.g. |
There was a problem hiding this comment.
The precision hint is only applied when the model is loaded. That works on x86, where the CPU plugin picks the precision again at import. It does not work on ARM. There, compile_model already converts the weights to f16 at export, and the exported blob keeps them. preprocess.py drops inference_precision, so the export always uses the default. We tried a 512x512 f32 matmul exported on an ARM CPU. With inference_precision set to f32, the compiled model reports f32, but the error is the same as plain f16: about 0.36, where real f32 gives about 4e-5. A blob exported on x86 does not load on ARM, so ARM users always hit this, and the new test line does not fix the tolerance failures on an ARM machine. Could preprocess.py also pass the value to compile_model as INFERENCE_PRECISION_HINT? On ARM that brought the error down to 4e-5. Please keep the load time call too, because on x86 a hint given only at export is lost at import. The new comment about picking the precision at import would then need to say it is about x86. GPU and NPU blobs are probably fixed at export as well.
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 17m 29s —— View job Besides the open inline threads, I found five minor issues: the CI script stops before the model tests when an op test fails, Detailed reviewPR Review: #24305SummaryThis PR adds an Code Quality
Infrastructure
API Design
Testing
RecommendationRequest Changes The main reason is the open inline threads:
The items above are minor and can be fixed in the same pass. |
The OpenVINO CI job runs backends/openvino/tests/test_runner.py. When a test failed, the runner printed "completed with failures" but still exited with code 0, so the CI job stayed green and the failure went unnoticed. Now the runner exits with code 1 when any test fails, so CI turns red. This is the test runner fix from #24305. Co-authored-by: suryasidd <surya.siddharth.pemmaraju@intel.com>
…#24294 (#24396) ## What is wrong today The OpenVINO partitioner gives `min`, `max`, `amin`, `amax`, `minimum`, `maximum`, `clamp` and `sign` to OpenVINO even when every tensor they read is bool. OpenVINO cannot convert these for bool: ``` ReduceMin ... Element type of data input must be numeric. Got: boolean ``` `torch.min(x > 0, dim=1)` and `MultivariateNormal`'s argument check (PyTorch 2.15) hit this. Reading masked values after writing them fails too: ```python x[mask] = x[mask] * 2 return x + x[mask].sum() # Dynamic rank for aten::index input is not supported. ``` ## What this change does The partitioner leaves these ops to ExecuTorch's CPU kernels when every tensor they read is bool. Ops that also read a float tensor, like `maximum(mask, x)`, stay with OpenVINO. Casting to uint8 could give wrong `min(dim)` and `max(dim)` indices on ties, as the docs now note. For the masked read, preprocess fills each node's `tensor_meta` from `val`. OpenVINO reads ranks only from `tensor_meta`, which ExecuTorch leaves empty. Follow-ups to #24294: - The test runner exits with 1 on failure (from #24305), and CI runs model tests even after an op test fails. - OpenVINO dropped a first input that only a folded `cumsum` read, failing every call. Preprocess puts it back and checks the input count. - An output planned as dynamic but compiled as fixed failed every call. Its buffer now starts with the compiled shape. - The `cumsum` fold works under a CUDA default device on a CPU-only PyTorch (exir's ScalarToTensorPass still fails there). - An empty last `inference_precision` entry now also means the default at load. - The docs say which CPUs compute some integer ops in float32, and where the float32 default is slower. ## What was tested Linux x86_64, OpenVINO 2026.4.1 and 2025.1.0, CPU, unless noted: - Op and model tests pass. The bool `min` model (both releases) and the masked read (2026.4.1) failed to lower, and now match eager. - On 2026.4.1, each backend fix has a test that fails without it. The runner and CI script were checked by hand. - Partitions are unchanged for every probed op with integer, float16, bfloat16 or float32 inputs, and for MobileNetV2, ResNet-18 and a transformer layer. - With the `tensor_meta` change, all other test partitions convert to the same OpenVINO graphs. - The docs' integer claims were measured on x86 through the delegate, and on ARM with plain OpenVINO 2026.4.1. --------- Co-authored-by: suryasidd <surya.siddharth.pemmaraju@intel.com>
Summary
Fixed 20 operator tests failing in the CI.
Test plan
Tested these tests locally and verified they are passing.