[shoumikhin]

What is wrong today

OpenVINO turns a copy like x.clone() into nothing, so a model returning such a copy returns its input directly, and the CPU plugin gives wrong values for it. A recent change made the partitioner run these copies on the CPU instead.

It also moved copies of weights and buffers to the CPU. Most did not need it, because a constant is stored inside the partition, not passed in as an input. That had costs:

class M(torch.nn.Module):
    def __init__(self):
        super().__init__()
        self.register_buffer("buf", torch.randn(256 * 1024))

    def forward(self, x):
        return self.buf.clone(), x + self.buf

• The 1 MiB buffer was stored twice, so the program grew to 2 MiB.
• With the buffer marked no_copy, export failed.
• With dim order off, self.buf.clone(memory_format=torch.preserve_format) failed at run time in the portable clone kernel.

Also, lowering a whole program with to_backend, without a partitioner, still gave wrong values for programs like return x.clone().

What this change does

• The partitioner leaves a copy of a constant inside the partition when:
  • only this partition reads the constant,
  • no other output of the partition is a copy of the same constant,
  • the copy keeps the default dim order,
  • the runtime can return its type, and it is not 64-bit, which the CPU plugin computes in 32 bits,
  • for a float copy, the precision is f32, the CPU default. At f16, OpenVINO on an ARM CPU rounded such a copy.
• For the CPU device, preprocess puts an OpenVINO Identity between an input and an output when the converted model would return the input directly. This fixes whole programs lowered with to_backend. The CPU plugin has no Identity before OpenVINO 2025.4, so with those releases such a program fails to lower, with an error naming the version it needs.

Not changed: a copy of a constant that still runs on the CPU can still have the costs above, as on main. With dim order off, x.clone(memory_format=torch.preserve_format) of a real input still fails. A whole program's int64 output outside the int32 range still comes back wrong, as for every int64 tensor on the CPU plugin (see the README).

Testing

• New tests in backends/openvino/tests/ops/test_copy.py check where copies run (including dim order off, uint16, f16 precision and 2 copies of one constant), mutated buffers, the error for OpenVINO before 2025.4 (version check mocked), and whole programs on the CPU with float, int64 and bool inputs and static and dynamic shapes. test_copy_of_constant_as_output and test_whole_program_returning_copy_of_input fail on main.
• All OpenVINO op tests pass on Linux x86_64 with OpenVINO 2026.4.1 and 2025.4.0. On 2025.3.0 the copy tests pass, and such a whole program gives the new error.
• Not tested on GPU or NPU.

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Planned maintenance affecting GB300 CI jobs

❌ 1 New Failure, 1 Unrelated Failure

As of commit e84a900 with merge base a3528d9 ():

NEW FAILURE - The following job has failed:

• pull / test-vulkan-operators-linux / linux-job (gh)
  RuntimeError: Command bash /__w/_temp/exec_script failed with exit code 127

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

• pull / nxp-mcuxpresso-test / linux-job (gh) (trunk failure)
  undefined reference to SYS_INFO_PrintAll'`

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[Copilot]

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]

@claude /pr-review

[claude]

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

---

Request Changes: a channels-last copy of a constant now stays in the OpenVINO partition and comes back with its values permuted. The review also asks why the PR keeps two fixes for the same aliasing bug, and it lists gaps in the new tests.

Detailed review

PR Review: #24506

Summary

The PR keeps copies of parameters, unmutated buffers and lifted tensor constants inside OpenVINO partitions. It also puts an Identity between each Parameter and Result in the converted model. Problems found:

• The exemption for constants ignores dim order, so a copy of a constant that changes the layout now returns wrong values.
• The PR keeps two mechanisms for one aliasing bug and doesn't say why.
• The new tests never run the mutated-buffer case, and they don't cover the dtypes or dynamic shapes the description lists.

Limits of this review: all four changed files were reviewed, by reading the code only, because Python could not run in this job. The checkout is shallow and origin/main has no merge base with this branch, so the diff was taken against the PR's parent, a3528d9.

Code Quality

• A copy of a constant that changes dim order now returns wrong values. backends/openvino/partitioner.py:98 exempts every copy of a constant, whatever its memory format.
  • Trigger: dim order on (the default), a 4-D buffer, and return self.buf.clone(memory_format=torch.channels_last), x + self.buf.
  • The _clone_dim_order now stays in the partition. DimOrderOpsRevertPass turns it into an aten.clone, which the converter drops.
  • The runtime builds output tensors from sizes only (create_ov_tensor, backends/openvino/runtime/OpenvinoBackend.cpp:728). OpenVINO writes the buffer in contiguous order into an output that ExecuTorch reads as channels-last. The values come back permuted, with no error.
  • Before this PR the copy ran on the CPU through _clone_dim_order, and the output was right.
  • test_copy_of_constant_as_output only uses a plain clone(), so it does not catch this.
  • Fix: keep the exemption only when the returned copy has the same dim order as the constant, and add a channels-last test case. Fix this →
• Two mechanisms for one aliasing bug, and the PR doesn't say why both stay. See backends/openvino/partitioner.py:81-109 and backends/openvino/preprocess.py:160.
  • _copy_returned_inputs runs in preprocess, so it applies to every partition, not only to whole-program lowering. Suppose a copy of a real input that keeps the layout stayed in a partition. It would become Parameter → Identity → Result, and test_whole_program_returning_copy_of_input shows that this gives the right values on CPU.
  • Moving those copies out to the CPU still has costs:
    • an extra CPU op for each such output;
    • the _skip_dim_order=True plus clone(memory_format=torch.preserve_format) failure, which the description lists as "Not changed";
    • _constants, which exists only to support the move-out.
  • The PR trusts Identity on every device for whole-program lowering, but not inside partitions.
  • Either move to the CPU only the returned copies that change dim order, whatever their source, or document why the move-out stays. The first option also covers the previous finding and removes the need for _constants. If the move-out is kept as a fallback for devices where Identity isn't trusted, say so, and note that whole-program lowering on those devices is still unprotected.
• _copy_returned_inputs only catches a Result fed directly by a Parameter (low confidence). See backends/openvino/preprocess.py:77.
  • The function's docstring says the CPU plugin can remove a Convert to the same type. A Parameter → Convert(same type) → Result from the converter gets no Identity, so once the plugin drops the Convert, the output aliases the input again.
  • The partitioner doesn't cover this case, because _to_dim_order_copy is not in _COPY_OPS.
  • Possible trigger: return x.to(torch.float32, copy=True), x.sin() with a float32 x, which reaches the converter as aten._to_copy(dtype=float32).
  • This depends on whether the converter emits a Convert for a _to_copy that keeps the dtype, which could not be checked here. One test through both paths would settle it.
• A copy of a constant that the CPU also reads stores the constant twice (minor).
  • backends/openvino/partitioner.py:98 checks the program-wide _constants, not whether every user of the constant is in this partition.
  • tag_constant_data ignores users without a tag (exir/backend/utils.py:382-395), so it tags the constant into the partition. _maybe_duplicate_constant_nodes then makes a second copy for the CPU user, or raises if the constant is marked no_copy (exir/backend/utils.py:229-237).
  • Trigger: return self.buf.clone(), op(self.buf) where op is not delegated. Before this PR the clone moved out, so no delegate read self.buf, and it was stored once.
  • Now it is stored twice, or export fails with no_copy. These are the two costs the description sets out to remove. Fix this →

Testing

• test_copy_of_mutated_buffer_as_output never runs the program. See backends/openvino/tests/ops/test_copy.py:89-97.
  • With assert_output_equal=False, execute_layer_test does not load or run the program (base_openvino_op_test.py:70). The test only checks where the clone is placed, so it would still pass if the CPU clone returned the value from after the mutation.
  • Checking values doesn't need the buffer's starting value. Run two inputs on one loaded method, then check out[1] ≈ sin(out[0]) for each call and out2[0] ≈ out1[0] + x1. Fix this →
• test_whole_program_returning_copy_of_input covers less than the description claims. See backends/openvino/tests/ops/test_copy.py:99-118.
  • It runs one call, with a float32 input of fixed shape, and loads the program from lowered.buffer().
  • The description says int and bool inputs and dynamic shapes were checked by hand. The _copy_returned_inputs docstring says those are the cases where a Convert to the same type broke, so they are the most likely to regress.
  • Test those dtypes, add an export with dynamic_shapes, and run a second input on the same method. Load the program from a file, as the harness does (base_openvino_op_test.py:74-80).
  • test_copy_of_constant_as_output could also pass more_inputs, so the constant output is checked across calls. Fix this →

Backward Compatibility

• Identity is inserted for every device, but only CPU was tested (low confidence). See backends/openvino/preprocess.py:160.
  • The aliasing it works around is described as a CPU plugin behavior, but the insertion runs whatever the device compile spec says.
  • If the GPU plugin or the NPU compiler has no Identity-16, whole-program lowerings that compile today, such as return x.clone(), would start failing in compile_model.
  • Before merging, run test_whole_program_returning_copy_of_input with backends/openvino/tests/test_runner.py --device GPU, and again with --device NPU. Otherwise, limit the insertion to the devices where it was checked.

Recommendation

Request Changes

The channels-last regression at partitioner.py:98 silently returns wrong values, and the fix is small. The choice to keep two mechanisms should be explained or simplified before more logic depends on _constants.

• Gather context (diff, commits, CLAUDE.md, checklist)
• Run independent reviews (3 reviewers)
• Consolidate findings
• Fact-check findings
• Post final review

[Copilot]

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]

Thanks for the review. The latest push addresses it:

• Channels-last copy of a constant: fixed. A copy of a constant now stays in the partition only if it keeps the default dim order. Otherwise it runs on the CPU, as before. The new channels-last test case fails without this.
• Constant also read on the CPU: fixed. The copy stays inside only when every user of the constant is in the partition, so the constant is stored once. A new test case uses an operator the partitioner does not take.
• Two mechanisms for one bug: kept on purpose. Running the copy on the CPU is exact on every device. Identity was only checked on the CPU plugin, so it is now added only for the CPU device. It is what fixes whole programs, which have no CPU part to fall back to. The description now says this.
• _to_copy that keeps the dtype: not reproduced. x.to(torch.float32, copy=True) is removed before the graph reaches OpenVINO, so there is no Convert for the plugin to drop. Through the partitioner, return x.to(torch.float32, copy=True), x.sin() gives right values.
• Mutated buffer test: fixed. It now runs 2 calls on one loaded method. It checks that the second call's copy equals the buffer value the first call left, and that each sin output matches its copy.
• Whole-program test: fixed. It covers float, int64 and bool inputs, a dynamic shape and a second call, and it loads the program from a file. The constant test also runs a second input.
• Identity on other devices: fixed. Only the CPU device gets it. Other devices get the converted model unchanged.

[shoumikhin]

@claude /pr-review

[claude]

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

---

Request Changes: a chain of copies of a constant can now be split across the OpenVINO partition boundary, so export fails with a dependency cycle. The tests also no longer cover the static-shape whole-program path, and on CPU they no longer check where copies of real inputs run.

Detailed review

PR Review: #24506

Summary

The PR keeps a returned copy of a constant inside the OpenVINO partition when every reader of the constant is in the partition and the copy keeps the default dim order. For the CPU device, it also puts an Identity between each Parameter and a Result it feeds directly. Problems found:

• The dim-order check looks at each copy in a chain separately. So a chain of copies of a constant can be split across the partition boundary, and export fails.
• A returned copy of an int64 constant now runs in OpenVINO, which computes int64 in 32 bits (low confidence).
• The whole-program test lost its static-shape case. On CPU, the new Identity also hides whether copies of real inputs still leave the partition.
• The constant exemption also applies on GPU and NPU, which were not checked. Identity was kept off those devices for exactly that reason.

Limits of this review: all four changed files were reviewed by reading the code only, because Python could not run in this job. The checkout is shallow and origin/main has no merge base with this branch, so the diff was taken against the PR's parent, a3528d9.

Code Quality

• A chain of copies of a constant can make the partition cyclic, so export fails. See backends/openvino/partitioner.py:99-118.
  • Trigger: y = self.buf.clone().clone(memory_format=torch.channels_last); return y, torch.sin(y). This combines the "clone of a clone" case of test_copy_of_input_as_output with the "channels last" case of test_copy_of_constant_that_runs_outside. Two contiguous copies do not trigger it.
  • The outer copy is returned and is channels-last, so copies_input takes it out. Line 106 checks the meta["val"] of the copy where the walk started, which here is the outer copy.
  • Taking it out makes the inner copy an output. The inner copy's own walk finds a contiguous copy of a constant that only the partition reads, so it stays.
  • Now the partition {inner clone, sin} feeds the outer clone on the CPU, and that clone feeds sin back in the partition. fuse_as_graphmodule, called from create_submodule_from_nodes (exir/lowered_backend_module.py:891), raises "Invalid partition, found dependency cycles".
  • On main, every copy in the chain left the partition, so this model exports.
  • Fix: when a copy leaves, take out the copies it reads too, and add this model as a test. Fix this →
• A returned copy of an int64 constant may now come back wrong (low confidence). See backends/openvino/partitioner.py:103-107.
  • backends/openvino/README.md:92-94 says the CPU plugin computes every int64 tensor in 32 bits, so a value outside the int32 range wraps around.
  • Trigger: register_buffer("ids", torch.tensor([2**40])) with return self.ids.clone(), x + 1. On main the copy ran on the CPU and was exact. Now it stays in OpenVINO.
  • This comes from the README, not from a run. If it holds, leave int64 copies out of the exemption and add a test with such a buffer. Fix this →

Testing

• The whole-program test no longer covers static shapes. See backends/openvino/tests/ops/test_copy.py:169.
  • Every subtest exports with Dim("rows"), so every output is dynamic. For a dynamic output, the runtime binds a tensor that OpenVINO owns and copies it out after inference (backends/openvino/runtime/OpenvinoBackend.cpp:546-560 and :579-598).
  • For a static output, the runtime binds ExecuTorch's own output buffer (OpenvinoBackend.cpp:540-545). That is the case the docstring at partitioner.py:87-89 describes, and the most common kind of export. The test as first added in d008d85 ran this path; the current version does not.
  • The d008d85 commit message says the CPU plugin handles a same-type Convert differently for dynamic shapes, so a test with one kind of shape does not cover the other.
  • Fix: also run each dtype with static shapes. Fix this →
• On CPU, the Identity hides whether copies of real inputs still leave the partition. See backends/openvino/tests/ops/test_copy.py:18-43.
  • test_copy_of_input_as_output only compares values. If a copy of an input stayed in the partition, _copy_returned_inputs would put an Identity on its Parameter → Result edge, and the values would still be right.
  • CI runs the op tests on CPU only (.ci/scripts/test_openvino.sh:17). So no test now checks that the aten.clone, full-slice and chain copies leave the partition. On other devices, moving them out is the only safeguard.
  • For example, removing aten.clone.default from _COPY_OPS would still pass every test on CPU.
  • Fix: also assert that the copy op is in execution_plan[0].operators, as the mutated-buffer test does. Fix this →
• The "also read outside" case depends on OpenVINO not supporting aten.atan (minor). See backends/openvino/tests/ops/test_copy.py:98-100.
  • If an OpenVINO release in the supported range adds atan, the copy stays inside and the assertIn at line 108 fails without saying why.
  • The partitioner's skip options cannot be used to force the op out. is_node_supported returns True for a supported op before it reads op_types_to_skip or op_names_to_skip (partitioner.py:193-202, existing code).
  • At least assert that aten::atan ran on the CPU too, so the failure explains itself. Fix this →

Backward Compatibility

• The constant exemption also applies on GPU and NPU, which were not checked (low confidence). See backends/openvino/partitioner.py:331-333.
  • preprocess keeps Identity off other devices because "neither the problem nor Identity was checked there" (preprocess.py:73-74). The exemption has no device check, so on GPU and NPU it now sends OpenVINO graphs that have only been run on the CPU plugin:
    • an output fed directly by a Constant;
    • a delegate with no inputs, for example from return self.buf.clone(), torch.atan(x). On main the lone copy left the partition and the partition was dropped. Now it lowers to a model with no Parameters, and no test covers that on any device.
  • If a plugin rejects either graph, models that lower today on GPU or NPU would start failing.
  • Fix: apply the exemption only for the CPU device (the partitioner has the compile specs in self.delegation_spec), or run the constant tests with test_runner.py --device GPU and --device NPU before merging. In both cases, add a test for the partition with no inputs. Fix this →

Recommendation

Request Changes

The cycle at partitioner.py:99-118 makes export fail for a model that exports on main, and the fix is small. The static-shape case should come back, and the tests should assert where copies of inputs run, so that CI, which runs on CPU only, still tests what this PR changes.

• Gather context (diff, commits, CLAUDE.md, checklist)
• Run independent reviews (3 reviewers)
• Consolidate findings
• Fact-check findings
• Post final review

[Copilot]

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]

Thanks. The latest push addresses this:

• Chain of copies splitting the partition: fixed. A copy that leaves the partition now takes the copies it reads with it. So buffer.clone().clone(memory_format=torch.channels_last) with torch.sin of the result exports again, as on main. A new test case covers it.
• int64 constant copies: confirmed and fixed. A copy of an int64 or float64 constant came back changed, because the CPU plugin computes 64-bit types in 32 bits. Such copies now run on the CPU. A new int64 test case covers it.
• Static shapes in the whole-program test: fixed. Each input type now runs with static and with dynamic shapes. The runtime treats them differently: for a static output it gives OpenVINO the ExecuTorch output tensor, and for a dynamic one it copies out a tensor OpenVINO owns.
• Where copies of inputs run: fixed. test_copy_of_input_as_output now also checks that the copy op runs on the CPU in each case.
• atan dependence: fixed. That case now also checks that aten::atan runs on the CPU, so a change in what the partitioner takes shows up clearly.
• Constant copies on GPU and NPU: kept as is. Before OpenVINO: fix wrong outputs when a partition returns a copy of its input #24466, every copy stayed in the partition on every device, so an output fed by a constant, or a partition that holds only a copy of a constant, was the old behavior everywhere. This change brings that back for the copies of constants it keeps. On the CPU, a partition that holds only such a copy, with no inputs, runs and gives right values. I did not add a test for it, since that was the behavior on every device before OpenVINO: fix wrong outputs when a partition returns a copy of its input #24466.

[shoumikhin]

@claude /pr-review

[claude]

Claude finished @shoumikhin's task in 25m 28s —— View job

---

Approve: I found no problems in the partitioner or preprocess changes, and the four minor problems left are all in the tests. No test covers the dim-order-off case the description says is fixed, the whole-program test also runs on devices where the fix is off, the new atan check runs too late to explain a failure, and the load-and-run block is copied twice.

Detailed review

PR Review: #24506

Summary

The PR keeps a returned copy of a constant inside the OpenVINO partition when OpenVINO can return it exactly. On the CPU device, it also puts an Identity between an input and an output that the converted model would otherwise return directly. The problems found are minor:

• No test covers a copy of a constant with dim order off. That is one of the three failures the description says this change fixes.
• The whole-program test also runs on GPU and NPU, where the fix it checks is off.
• In the "also read outside" case, the atan check runs after the check whose failure it is meant to explain.
• The new tests copy the harness's load-and-run block twice.

Limits of this review: I reviewed all four changed files by reading the code only, because Python could not run in this job. The checkout is shallow and origin/main has no merge base with this branch, so I took the diff against the PR's parent, a3528d9.

Code Quality

• The new tests copy the harness's load-and-run block twice. See backends/openvino/tests/ops/test_copy.py:155-164 and :204-215.
  • Both copy the block in execute_layer_test (base_openvino_op_test.py:76-85). It writes the program to a file, loads it, runs inputs on one method, and then deletes the method and program so Windows can delete the file.
  • Those steps now live in three places. A later change to one of them, such as the Windows workaround, is easy to miss in the others.
  • Fix: move the block into a method on BaseOpenvinoOpTest that runs a list of inputs on one loaded method and returns the outputs. execute_layer_test and both tests can then call it. Fix this →

Testing

• No test covers a copy of a constant with dim order off. See backends/openvino/tests/ops/test_copy.py:61-86.
  • The description lists three failures this change fixes. In the third, with dim order off, self.buf.clone(memory_format=torch.preserve_format) failed at run time. The portable clone kernel accepts only no memory format or contiguous_format (kernels/portable/cpu/op_clone.cpp:44-51).
  • Both constant tests run only with the default EdgeCompileConfig. Only test_copy_of_input_as_output turns dim order off, and only for a real input. So no test runs a copy of a constant as aten.clone, which is what LLM exports produce.
  • Fix: add a subtest to test_copy_of_constant_as_output that returns self.constant.clone(memory_format=torch.preserve_format), with compile_config=EdgeCompileConfig(_skip_dim_order=True). By my reading of the parent's partitioner, this case fails on the parent and passes with this change.
  • The description's "Not changed" note also names only real inputs. Commit de572d8 says that a copy of a constant that still leaves the partition fails the same way with dim order off. That covers a 64-bit copy, and a copy of a constant that the CPU also reads. Fix this →
• The whole-program test also runs on GPU and NPU, where the fix is off. See backends/openvino/tests/ops/test_copy.py:170.
  • _copy_returned_inputs returns early unless the device starts with CPU (preprocess.py:76). But the test lowers for self.device (line 199), which test_runner.py --device sets (backends/openvino/tests/test_runner.py:20-21).
  • With --device GPU or --device NPU, the test runs a path this PR leaves unchanged. If that device has the problem, the test fails. If it doesn't, the test passes without testing the fix.
  • Fix: skip the test when the device is not the CPU, as test_addmm.py:39-40 does. Fix this →
• The atan check runs after the check whose failure it is meant to explain. See backends/openvino/tests/ops/test_copy.py:127-129.
  • The check was added so that "a change in what the partitioner takes shows up clearly". But if an OpenVINO release supports atan, the clone stays in the partition. Then the assertIn for dim_order_ops::_clone_dim_order at line 127 fails first, and the aten::atan check at line 129, which names the cause, never runs.
  • Fix: in that case, check for aten::atan before the clone. Fix this →

Recommendation

Approve

Nothing here blocks merging. Each fix above is a small change to the tests, so they can go into this PR or a follow-up.

• Gather context (diff, commits, CLAUDE.md, checklist)
• Run independent reviews (3 reviewers)
• Consolidate findings
• Fact-check findings
• Post final review

[Copilot]

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.

[Copilot]

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]

@claude /pr-review

[claude]

Claude finished @shoumikhin's task in 26m 46s —— View job

---

Approve: one minor problem is left: a copy of a constant that still runs on the CPU still makes the program store the constant twice, or fail to export with no_copy, the description doesn't say so, and two of these copies could stay in the partition.

Detailed review

PR Review: #24506

Summary

The PR keeps a returned copy of a constant inside the OpenVINO partition when OpenVINO can return it exactly. On the CPU device, it also puts an Identity between an input and an output that the converted model would otherwise return directly. One minor problem was found:

• When a copy of a constant still runs on the CPU and a partition also reads the constant, the constant is still stored twice. The description doesn't list this, and two of these copies don't need to leave.

Limits of this review: I reviewed all four changed files by reading the code only, because Python could not run in this job. The checkout is shallow and origin/main has no merge base with this branch, so I took the diff against the PR's parent, a3528d9.

Code Quality

• A copy of a constant that still runs on the CPU still stores the constant twice, and the description doesn't say so. See backends/openvino/partitioner.py:104-124.
  • Suppose such a copy leaves while the partition also reads the constant. tag_constant_data tags the constant into the partition. _maybe_duplicate_constant_nodes then makes a second copy for the CPU, or raises if the constant is marked no_copy (exir/backend/utils.py:228-237).
  • A channels-last or 64-bit copy does this, as in the "channels last" case at backends/openvino/tests/ops/test_copy.py:116-119. Commit de572d8 says so, but the "Not changed" section of the description lists only the preserve_format failure. The description names the double storage and the no_copy failure as what this PR fixes, so these cases belong in "Not changed".
  • Two more kinds of copies leave even though keeping them would be safe. Both behave as on main:
    • Line 122 always takes the inner copies of a chain out with the outer copy. A cycle can only form when the copy that leaves feeds the partition. Trigger: return self.buf.clone().clone(memory_format=torch.channels_last), x + self.buf. Nothing inside reads the outer copy, so the inner clone could stay as a partition output, and buf would be stored once.
    • Line 106 requires every reader of the constant to be in this partition. But _maybe_duplicate_constant_nodes already gives each partition that reads a constant its own copy, so only readers on the CPU add storage. Trigger: return self.w.clone(), torch.atan(x @ self.w) @ self.w, where atan splits the two matmuls into two partitions. The clone leaves, so the CPU stores one more copy of w.
  • Either narrow these two conditions or list them in "Not changed" too. Fix this →

Recommendation

Approve

Nothing blocks merging. Updating the description is enough. Narrowing the two conditions is optional.

• Gather context (diff, commits, CLAUDE.md, checklist)
• Run independent reviews (3 reviewers)
• Consolidate findings
• Fact-check findings
• Post final review

[Copilot]

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]

Thanks. The latest push updates the description and fixes a few more cases:

• Copies of a constant that still run on the CPU: now listed under "Not changed". They can still store the constant twice, or fail with no_copy, as on main. That includes the 2 cases you found: an inner copy in a chain, and a constant that another partition also reads. I left them as they are to keep this change small.
• Also fixed: a partition that returned 2 copies of one constant failed to lower, and a uint16 copy of a constant failed at run time. Both now run on the CPU, as on main. With OpenVINO before 2025.4, whose CPU plugin has no Identity, a whole program that returns a copy of its input now fails with an error that names the version it needs, instead of "Unsupported operation of type: Identity".
• Minimum OpenVINO version: I kept 2025.1. From 2025.3 on, OpenVINO's Linux aarch64 wheels need glibc 2.35, so a higher minimum would stop the openvino extra from installing on older aarch64 systems, such as Ubuntu 20.04.

One known limit: a float32 copy of a constant that stays in the partition loses subnormals and the sign of zero, so [1e-40, -0.0] comes back as [0, 0]. The CPU plugin does the same in other float32 ops, such as x + b and x * b, with or without this change.

[shoumikhin]

@claude review this PR

[claude]

Claude finished @shoumikhin's task in 4m 8s —— View job

---

Approve: I found no blockers, but two minor problems are left: the whole-program int64 test uses only small values, so it can't catch the 32-bit truncation the partitioner already guards against, and a kept float32 copy of a constant can come back rounded when a lower inference_precision is set (low confidence).

Detailed review

PR Review: #24506

Summary

The PR keeps a returned copy of a constant inside the OpenVINO partition when OpenVINO can return it exactly. On the CPU device, it also puts an Identity between an input and an output that the converted model would otherwise return directly. Problems found:

• The whole-program path puts an Identity on 64-bit inputs, which the partitioner's own rule says the CPU plugin computes in 32 bits. The test only uses values that fit in int32 (low confidence).
• The rule for keeping copies of constants doesn't look at inference_precision, so with f16/bf16 a returned copy of a float32 weight may not be exact (low confidence).
• Three smaller problems: AUTO/HETERO/MULTI devices skip the Identity, and the version parse can raise a confusing ValueError.

Limits of this review: I reviewed all four changed files by reading the code only, because Python and OpenVINO could not run in this job. So every claim below about what an OpenVINO plugin does is suspected, not confirmed. The checkout is shallow and origin/main has no merge base with this branch, so I took the diff against the PR's parent, a3528d9.

Code Quality

• Identity on a 64-bit input may truncate it, and the test can't tell (low confidence). See backends/openvino/preprocess.py:98 and backends/openvino/tests/ops/test_copy.py:203.
  • _CONSTANT_COPY_DTYPES (partitioner.py:36-37) leaves out int64 and float64 because "the CPU plugin computes 64-bit types in 32 bits". Commit 6f44ca6 confirmed that a copy of torch.tensor([2**40]) came back changed.
  • _copy_returned_inputs adds an Identity for every dtype. If the plugin also runs that Identity in 32 bits, a whole program return x.clone() with an int64 input of 2**40 (or a float64 input that needs more than float32 precision) gives a truncated value.
  • On the parent, this output aliased the input and was already wrong, so this is not a regression. But the description says the int64 whole-program case is fixed.
  • The constant test uses 2**40 (test_copy.py:131), but the whole-program int64 subtest uses torch.randint(-100, 100, ...), which hides this.
  • Fix: use values outside the int32 range in the whole-program int64 subtest, and add float64. If they fail, say in the description that 64-bit whole programs still lose precision, or have preprocess reject them the way it rejects pre-2025.4 releases. Fix this →
• A kept float32 copy of a constant can come back rounded under reduced precision (low confidence). See backends/openvino/partitioner.py:136-144.
  • The rule checks dtype, dim order and readers. It doesn't check inference_precision, which preprocess.py:189 passes through as INFERENCE_PRECISION_HINT. The GPU plugin also defaults to f16.
  • Trigger: inference_precision="bf16" or "f16" on the CPU, with return self.w.clone(), x + self.w for a float32 parameter. If the plugin stores or computes the Constant → Result in that precision, the returned "copy" is rounded. On the parent, the copy ran on the ExecuTorch CPU and was bit-exact.
  • The user did ask for lower precision for compute, but a copy of a weight changing value is still surprising, and the kept-copy case was only checked with f32.
  • Fix: keep float copies of constants only when the precision is f32, using the compile specs in self.delegation_spec. Otherwise, run one test with inference_precision="f16" to show the copy stays exact.

Backward Compatibility

• AUTO, HETERO:CPU and MULTI:CPU skip the Identity (minor). See backends/openvino/preprocess.py:88.
  • device.startswith("CPU") misses virtual devices that can run on the CPU plugin. With AUTO, early calls usually run there. A whole program that returns x.clone(), lowered with to_backend and device="AUTO", still gives the wrong values this PR fixes for CPU.
  • The precision rule at line 189 has the same pattern, so this matches the file. If it's left as is, say in the docstring that only CPU device strings are covered.
• The version parse can raise a ValueError that hides the real error (minor). See backends/openvino/preprocess.py:73.
  • int(part) on the first two dot-separated fields of get_version() fails for a source or custom build whose version string doesn't start with numeric major.minor.
  • It only runs when lowering is about to need Identity, but the user then gets ValueError: invalid literal for int() instead of the version message.
  • Fix: parse with packaging.version and fall back to trying Identity on parse failure, or catch the error and raise the intended message.

Recommendation

Approve

Nothing blocks merging. The int64 test change is small and would show whether the description's claim about 64-bit whole programs holds. The precision question needs one run to settle.

• Gather context (diff, commits, CLAUDE.md, checklist)
• Run independent reviews (3 reviewers)
• Consolidate findings
• Fact-check findings
• Post final review

[Copilot]

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]

Thanks. Answers below.

• Float copies at a lower precision: fixed. A float copy of a constant now stays in the partition only when the precision is f32, the CPU default. On an x86 CPU with bfloat16 units the copy came back exact under every precision. But on an ARM CPU at f16, OpenVINO returned such a constant rounded (3.14159265 as 3.140625), and the README suggests f16 there. On other devices, where the backend sets no default precision, float copies now run on the CPU unless f32 is set. Tests check that at f16 a float copy runs on the CPU and an int32 copy stays.
• Large int64 values in the whole-program test: you are right that they fail. [2**40, -2**40, 5] comes back as [0, 0, 5], and the reshaped output of the same program is wrong too. The CPU plugin computes every int64 tensor in 32 bits, as the README says, so Identity truncates like any other op. So I kept the test inside the int32 range and listed this under "Not changed". A float64 copy came back exact, so it needs no note.
• AUTO, HETERO and MULTI: left as they are. I did not check the problem or Identity on those devices, and the docstring says so. The same CPU check already picks the default precision.
• Version parsing: OpenVINO's build stops with an error unless the version starts with numbers for major, minor and patch, and a source build gets one too (for example 2026.4.1-22982-...). So int() does not fail on them.

[shoumikhin]

@claude review this PR

[claude]

Claude finished @shoumikhin's task in 4m 54s —— View job

---

Approve: I found nothing that blocks merging. One minor test gap is left: six of the eight types whose constant copies now stay in the partition never run in a test.

Detailed review

PR Review: #24506

Summary

The PR keeps a returned copy of a constant inside the OpenVINO partition when OpenVINO can return it exactly. On the CPU device, it also puts an Identity between an input and an output that the converted model would otherwise return directly. I found one minor problem: the tests cover only some of the types that the new rule keeps in the partition.

Limits of this review: I reviewed all four changed files by reading the code only, because Python and OpenVINO could not run in this job. The checkout is shallow and origin/main has no merge base with this branch, so I took the diff against the PR's parent, a3528d9.

I left out points the author has already answered in this thread: constant copies on GPU and NPU, a partition with no inputs, the AUTO, HETERO and MULTI devices, and version parsing.

Testing

• Most of the types that a kept constant copy can have are never tested. See backends/openvino/partitioner.py:41-50 and backends/openvino/tests/ops/test_copy.py:60-111.
  • _CONSTANT_COPY_DTYPES keeps copies of bool, uint8, int8, int16, int32, float16, bfloat16 and float32 constants in the partition. The "stays inside" tests use only float32 and int32.
  • For the other six types, no test runs an output fed directly by a Constant. These types are on the list because the runtime binds them (runtime/OpenvinoBackend.cpp:783-801), not because a run showed their values come back exact.
  • The 64-bit, uint16 and f16-precision cases each turned out to need a special rule only once they were run. The float16 and bfloat16 copies under the f32 hint are the most likely to be converted inside the CPU plugin, especially on ARM.
  • Fix: add the remaining types as subtests of test_copy_of_constant_as_output, using CopyOfConstant("buffer", dtype=...) as the int32 case does. Leave out any type that fails, as was done for uint16. Fix this →

Recommendation

Approve

Nothing blocks merging. The extra subtests are small and can go into this PR or a follow-up.

• Gather context (diff, commits, CLAUDE.md, checklist)
• Run independent reviews (3 reviewers)
• Consolidate findings
• Fact-check findings
• Post final review