Skip to content

OpenVINO: fix wrong outputs when a partition returns a copy of its input - #24466

Merged
shoumikhin merged 1 commit into
pytorch:mainfrom
shoumikhin:ov-passthrough-output
Nov 2, 2026
Merged

shoumikhin merged 1 commit into
pytorch:mainfrom
shoumikhin:ov-passthrough-output

Conversation

@shoumikhin

@shoumikhin shoumikhin commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

What is wrong today

When an OpenVINO partition returns a copy of one of its inputs, ExecuTorch gets stale memory back. For example:

class M(torch.nn.Module):
    def forward(self, x):
        y = x.clone()
        return y, torch.sin(y)

Lowered with OpenvinoPartitioner for CPU, y holds whatever its buffer held before, and torch.sin(y) is wrong too. OpenVINO can even overwrite x, so later ops that read x go wrong as well. The same happens for x.detach(), x[:, :], and chains like x.clone().clone(). This made a robot policy exported with LeRobot return wrong actions.

The cause is in OpenVINO. Its PyTorch frontend converts aten.clone and aten.alias_copy to nothing, so the converted model returns its input directly. ExecuTorch's runtime sets its output tensors after its input tensors. In that order the CPU plugin does not write such an output, and it can overwrite the input with the old contents of the output buffer. A plain OpenVINO script, without ExecuTorch, shows the same problem in every release tried, from 2025.2.0 to 2026.4.1 and a 2026.5 nightly.

What this change does

The partitioner leaves such a copy out of the partition when its result leaves the partition, so ExecuTorch's CPU kernel makes the copy. This covers aten.clone, aten.alias_copy, and dim_order_ops._clone_dim_order (which becomes aten.clone before conversion), including chains of them. A copy used only inside the partition stays there. OpenVINO skips it anyway, and taking it out could hand OpenVINO an input in a dim order it does not read.

Not touched here: the backend still reads and writes partition inputs and outputs in the default dim order, so a channels-last tensor at the partition edge can give wrong values.

Setting the output tensors first also avoids the bug, but only through an undocumented detail of the plugin, so this change does not rely on it.

With dim order turned off, a channels-last copy of an input that leaves the partition now fails at export instead of giving wrong values.

execute_layer_test gets an optional compile_config, to test lowering with dim order turned off, where a clone is aten.clone.

Testing

  • New tests in backends/openvino/tests/ops/test_copy.py. test_copy_of_input_as_output fails without the partitioner change and passes with it. test_copy_of_input_used_inside checks that a copy used only inside the partition stays there.
  • All OpenVINO op tests pass on Linux x86_64 with OpenVINO 2026.4.1.
  • The robot policy above now exports to OpenVINO and passes its accuracy check.

@shoumikhin shoumikhin added the release notes: openvino OpenVino backend related issues, features, bugs etc. label Nov 2, 2026
Copilot AI balanced review requested due to automatic review settings November 2, 2026 21:11
@pytorch-bot

pytorch-bot Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24466

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures, 6 Pending, 1 Unrelated Failure

As of commit e71822f with merge base 893d815 (image):

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 2, 2026
@shoumikhin

Copy link
Copy Markdown
Contributor Author

@claude /pr-review

@claude

claude Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Claude finished @shoumikhin's task in 19m 36s —— View job


The partitioner change fixes the reported stale-output case, but when a channels-last copy also has users inside the partition, those users now read its result in the wrong layout and give wrong values without any error, so this needs changes before merging.

Detailed review

PR Review: #24466

Summary

The PR makes OpenvinoPartitioner leave a clone, alias_copy or _clone_dim_order of a partition input out of the partition when the copy's result leaves the partition, so OpenVINO never returns an input directly as an output. One case still gives wrong values without an error: a copy used both inside and outside the partition. Also, the hand-kept list of copy ops has no check that would catch an op it misses. All three changed files were reviewed. Python could not run in this job, so these findings come from reading the code, not from running it.

Code Quality

  • A copy with users both inside and outside the partition is taken out, and the users left inside read its result in the wrong layout. backends/openvino/partitioner.py:78-84

    • Line 81 takes a copy out if any of its users is outside the partition, even when others are inside. The users left inside then receive the copy made on the CPU as a delegate input.
    • Nothing on that path looks at dim order. preprocess.py:110-117 gives the converter only shape and dtype. create_ov_tensor (runtime/OpenvinoBackend.cpp:728-777) builds the OpenVINO tensor from sizes() and mutable_data_ptr() alone.
    • The docstring (lines 66-67) and test_copy.py:42-43 describe exactly this risk, but the code avoids it only for copies used only inside the partition.
    • Trigger (default dim order): y = x.clone(memory_format=torch.channels_last); return y, y.mean(dim=(2, 3)). With this PR y is right, but mean reads NHWC data as NCHW and gives wrong values without an error. Before this PR, OpenVINO skipped the clone and mean read x.
    • Chains do the same: in a = x.clone(memory_format=torch.channels_last); b = a.clone(); return a, b.sum(1), a is taken out and b (used only inside) stays, so OpenVINO reads a's channels-last buffer.
    • Not covered by the new tests: sin is elementwise, so the layout does not change its result.
    • Fix: make this fail at export. For example, OpenvinoBackend.preprocess could reject a partition input whose meta["val"] is not in the default dim order. That would also catch any other way a non-default layout reaches the delegate. Add the model above to test_copy.py. Fix this →
  • _COPY_OPS copies OpenVINO's conversion rules by hand, and nothing catches an op it misses. backends/openvino/partitioner.py:28-33

    • Any other op that OpenVINO turns into an identity brings back the same stale output, again with no error. preprocess has no check that would turn it into an export error.
    • Likely miss (low confidence, not reproduced): dim_order_ops._to_dim_order_copy.default.
      • This partitioner delegates it (line 90).
      • DimOrderOpsRevertPass (preprocess.py:98) turns it into aten._to_copy with only memory_format and adds no dtype (exir/passes/memory_format_ops_pass.py:170-173).
      • As far as I know, OpenVINO's FX _to_copy translator adds a Convert only when a dtype is given.
      • If so, y = x.to(memory_format=torch.channels_last); return y, y.sin() hits the bug this PR fixes.
    • Suggestion: right after frontend.convert(...) (preprocess.py:143), raise if any result of the converted model comes directly from a Parameter. An op missing from the list then fails at export instead of returning stale memory. If the example above reproduces, also add _to_dim_order_copy and _to_copy without a dtype change to _COPY_OPS. Fix this →
  • Unneeded defensive getattr at backends/openvino/partitioner.py:53.

    • getattr(node.target, "__name__", None) is the dynamic getattr pattern CLAUDE.md asks to avoid.
    • The None default is never needed: only call_function nodes get into a partition (lines 137-138), and the same file reads node.target.__name__ directly (lines 142, 269).
    • Fix this →

Testing

  • backends/openvino/tests/ops/test_copy.py:20-24: the description lists x.detach() as an affected case, but copies has no detach entry. So no test checks that _COPY_OPS covers the op it lowers to. Fix this →

Recommendation

Request Changes

The case of a copy used both inside and outside the partition now gives wrong values without an error. The PR's own docstring describes this risk. It is cheap to turn into an export error and to cover with a test. The check that no output comes directly from an input is optional, but it would make any op missing from _COPY_OPS fail at export instead of giving wrong values.

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@suryasidd @cavusmustafa @ynimmaga @anzr299 @daniil-lyakhov could you take a look at this one?

It fixes wrong results when an OpenVINO partition returns one of its inputs unchanged, for example x.clone(). In that case the OpenVINO CPU plugin does not write the output into the buffer we bind, so the output keeps stale memory. When that input also feeds other ops, its own buffer gets overwritten too. This change keeps such copies on the CPU, so OpenVINO never sees a model whose output is just an input. With it, the eo1 policy from LeRobot goes from a max error of 2.06 to 0.063.

The bug also reproduces in plain OpenVINO (2025.2, 2026.4.1 and nightly), with no ExecuTorch involved. Binding the outputs before the inputs avoids it, which may help find the root cause.

Thanks!

When an OpenVINO partition returned a copy of one of its inputs, for
example `x.clone()` or `x[:, :]`, the copy came back with stale memory,
and so did anything else the partition computed from that input.

OpenVINO converts these copies to nothing, so the converted model returns
its input directly. ExecuTorch's runtime binds the output buffers after
the inputs, and in that order OpenVINO's CPU plugin does not write such an
output and can overwrite the input with the output buffer's old contents.
Every OpenVINO release tried behaves this way, from 2025.2 to the current
nightly. Binding the outputs first avoids it, but only because of an
undocumented detail of the plugin.

The partitioner now leaves such a copy (aten.clone,
dim_order_ops._clone_dim_order or aten.alias_copy) out of the partition
when its result leaves the partition, so it runs on the CPU kernels.
Copies used only inside the partition stay there, since OpenVINO skips
them anyway.

The OpenVINO op tests can now pass an EdgeCompileConfig, to also cover
lowering with dim order turned off, which gives aten.clone.
@shoumikhin
shoumikhin force-pushed the ov-passthrough-output branch from ce2f519 to e71822f Compare November 2, 2026 21:58
Copilot AI balanced review requested due to automatic review settings November 2, 2026 21:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@shoumikhin

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Answers by item:

  1. Copy used both inside and outside the partition. I ran your model, y = x.clone(memory_format=torch.channels_last); return y, y.mean(dim=(2, 3)). On main both outputs are wrong: OpenVINO overwrites x too, so mean does not read x either. With this change y is right and only mean is wrong. That remaining error is a separate problem. The OpenVINO backend reads and writes every partition input and output in the default dim order, so a channels-last tensor at the partition edge can give wrong values today, with or without this change. For example, (x * 2).to(memory_format=torch.channels_last) returned from a partition is wrong on main too. I'd rather fix that on its own than grow this change.

  2. _to_dim_order_copy and the op list. OpenVINO converts _to_dim_order_copy to a Convert node, not to nothing, so it does not hit this bug. Your example gives the same wrong values on main and with this change, for the dim order reason above. The list covers every op we found that OpenVINO turns into nothing, out of about 100 identity-like graphs tried, so I'd keep this change to the fix itself.

  3. getattr and detach. I replaced the getattr with the same type check is_node_supported uses. x.detach() lowers to aten.alias_copy, the same op as the full slice the tests already cover, so a separate case would test the same path twice.

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@claude /pr-review

@claude

claude Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Claude finished @shoumikhin's task in 23m 30s —— View job


No blocking issues found. Two text fixes remain: the PR description says both new tests fail without the change, but one passes on main, and the workaround's docstring links no upstream OpenVINO issue.

Detailed review

PR Review: #24466

Summary

The PR leaves a clone, alias_copy or _clone_dim_order of a partition input out of the OpenVINO partition when the copy's result leaves the partition, so OpenVINO never returns an input directly as an output. Two minor problems remain. Both are in text, not in behavior. All three changed files were reviewed. Python could not run in this job and the OpenVINO sources were not available, so these findings come from reading the ExecuTorch code.

Code Quality

  • The PR description says the new tests fail without the partitioner change, but test_copy_of_input_used_inside passes on main. backends/openvino/tests/ops/test_copy.py:37-44

    • The clone's only user is mean, which is inside the partition. So main and this PR both keep the clone in the partition, OpenVINO skips it, and mean reads the contiguous x.
    • The test checks that copies used only inside the partition stay there. Say so in the description, since the description becomes the commit message.
  • The workaround links no upstream OpenVINO issue. backends/openvino/partitioner.py:59-68

    • The docstring describes a CPU plugin bug. Per the PR description, it reproduces in plain OpenVINO from 2025.2.0 through a 2026.5 nightly.
    • Without an upstream report, there is no way to tell when OpenVINO fixes it and _remove_copies_of_inputs can be removed.
    • File the plain-OpenVINO repro upstream and link it in the docstring.

Recommendation

Approve

Neither point blocks merging. Both are text fixes, one to the PR description and one to the docstring.

@suryasidd

Copy link
Copy Markdown
Collaborator

@suryasidd @cavusmustafa @ynimmaga @anzr299 @daniil-lyakhov could you take a look at this one?

It fixes wrong results when an OpenVINO partition returns one of its inputs unchanged, for example x.clone(). In that case the OpenVINO CPU plugin does not write the output into the buffer we bind, so the output keeps stale memory. When that input also feeds other ops, its own buffer gets overwritten too. This change keeps such copies on the CPU, so OpenVINO never sees a model whose output is just an input. With it, the eo1 policy from LeRobot goes from a max error of 2.06 to 0.063.

The bug also reproduces in plain OpenVINO (2025.2, 2026.4.1 and nightly), with no ExecuTorch involved. Binding the outputs before the inputs avoids it, which may help find the root cause.

Thanks!

Hey @shoumikhin, i have reproduced the bug, i will raise it with the core OpenVINO team. Should I create another PR to fix the other issue about non-default dim order ?

@shoumikhin
shoumikhin merged commit 81a4666 into pytorch:main Nov 2, 2026
218 of 221 checks passed
@shoumikhin

Copy link
Copy Markdown
Contributor Author

@pytorchbot cherry-pick --onto cherry-pick-24396-by-pytorch_bot_bot_ -c critical

@pytorchbot

Copy link
Copy Markdown
Collaborator

Cherry picking #24466

The cherry pick PR is at #24478 and it is recommended to link a critical cherry pick PR with an issue.

Details for Dev Infra team Raised by workflow job

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: openvino OpenVino backend related issues, features, bugs etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants