Repository navigation
Fix 0-d tensors, dynamic output sizes and integer reductions, and default to float32 on CPU, in the OpenVINO backend - #24294
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24294
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit cc9b095 with merge base b50c571 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
ca054b6 to
b9d75ec
Compare
b9d75ec to
3fb1360
Compare
3fb1360 to
bc88265
Compare
bc88265 to
03e2941
Compare
03e2941 to
8969a7b
Compare
8969a7b to
89f775c
Compare
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 38m 1s —— View job Summary: The new float32-on-CPU default slows down programs that were exported before it on x86 CPUs with bf16 units, and neither a load-time option nor the repo's own export tools can switch it off; the other findings are minor issues in the cumsum-folding pass, the output-resize loop, the tests and the docs. Detailed reviewPR Review: #24294SummaryThis PR fixes 0-d tensors, dynamic output sizes and integer reductions in the OpenVINO backend, and makes float32 the default inference precision on CPU. That default needs discussion before merge, for three reasons:
All other findings are minor. Code Quality
Infrastructure
Testing
API Design
Backward Compatibility
Performance
RecommendationNeeds Discussion. Approval waits on decisions about the precision default:
All other findings are minor.
Not done: nothing was built or run. The claims about OpenVINO's behavior come from reading the code, and the ones I couldn't confirm are marked low confidence. |
PyTorch computes sum, cumsum and prod of a bool or integer tensor in
int64. OpenVINO's PyTorch frontend keeps a narrower type for some of
them. A cumsum over a bool mask is computed in 8 bits, so the count
wraps after 255. A cumsum or prod over int32 returns int32. When such a
result is a delegate output, the runtime rejects it at execute:
m = torch.rand(2, 7) > 0.5
torch.cumsum(m, 1) # CALL_DELEGATE execute failed: 0x1
Inside a graph the wrong values flow on silently. pi0 and pi0.5
compute their position ids as cumsum(pad_masks) - 1 over more than
255 tokens, so every position from 255 on was wrong and the policies
gave wrong actions.
Cast the input of these reductions to their result dtype before the
graph goes to OpenVINO. The open pytorch#24024 proposes a similar cast for
bool sum and cumsum.
The cast exposes an OpenVINO CPU bug, seen on 2025.4.1, 2026.0.0 and
2026.4.1 but not on the nightly: with an int64 cumsum over a constant,
a loaded program gives wrong results from the second inference on.
pi0 builds its attention masks from such a cumsum, and each call's mask
kept only what earlier calls allowed. So the backend now replaces an
int64 cumsum that reads only constants with its result before
converting. Buffers the graph writes to, random ops and higher-order ops
are not treated as constants, and other constants, such as compressed
weights, are left as they are. The fold computes on the CPU, so lowering
does not need the GPU, or the pinned memory, a constant was created
with. If the CPU cannot compute such a constant, the cumsum is left to
OpenVINO as before.
The new tests run cumsum over bool and int32, prod over int32, and
pi0-style attention masks and positions over 816 tokens through one
loaded program twice, with more valid tokens the second time. Pass tests
check that mutated buffers, random draws (direct and inside cond) and
float cumsums are not folded, that cumsums over a split constant and
over CUDA and pinned constants without a GPU are, and that a folded
result does not replace a buffer.
A bad inference_precision value was only reported after the whole graph had been converted, which takes a while for a large model. The check now runs first. The test makes conversion fail if it starts, so it fails when the check comes after conversion.
FoldConstantCumsumPass evaluated every constant int64 cumsum in one go. If any of them could not be evaluated on the CPU, none were folded, and lowering only logged a generic warning. The unfolded cumsums then give wrong results from the second inference on with the affected OpenVINO CPU plugin. The pass now evaluates each cumsum separately, so one failure no longer keeps the others from being folded, and the warning names the cumsum and says that its results can be wrong.
Add the cases the earlier tests left out: - sum over a bool mask with more than 255 true values in a row, and over int32, which must come out in int64 as in PyTorch; - a full prod (prod.default) next to prod over one dimension; - 0-d tensors as inputs of the delegate, not only as outputs; - the earlier static output computed from a data dependent view, kept next to the new dynamic output case. The float32 addmm test now skips on CPUs where OpenVINO computes in float32 anyway, so a runner that cannot check the default reports a skip instead of a pass.
The Precision section listed which x86 instruction sets give bfloat16 and how CPUs without bfloat16 units simulate it. That is OpenVINO's behavior and changes with its releases, so link to OpenVINO's Precision Control page instead, and keep only what the f16 and bf16 values do on CPUs without float16 and on ARM. Also say: - that the float32 default applies when the device is CPU, and also to programs exported in float16; - that bf16 brings back the old speed of an older program on x86, and that on ARM an older program keeps float16 until it is exported again; - that OpenVINO's CPU plugin computes int64 in 32 bits, so an integer result past the int32 range wraps without an error, including the int64 result of cumsum and prod over smaller integer types.
CastReductionInputPass also cast the input of sum to int64. OpenVINO's PyTorch frontend already does that for sum, so the extra cast changed nothing. Only cumsum and prod need it. test_sum, over a bool mask with more than 255 true values in a row and over int32, passes with and without the cast, and keeps checking that a sum comes out in int64 as in PyTorch.
To let an output come back smaller than its upper bound, the backend recorded each output's size at the first call on a delegate handle, and restored it before every later call. Programs from ExecuTorch 0.7 and earlier run every call of a lowered module on one handle, each with its own output tensors, so a second call with a larger output than the first failed: OpenVINO cannot write more than a borrowed buffer's shape holds. An output whose size can change now goes to a buffer that OpenVINO owns and grows as needed, one per output, made on first use and reused by every call. After inference, the backend resizes the ExecuTorch output to OpenVINO's shape and copies the data in. Each call overwrites that buffer, so nothing from one call reaches the next. Static outputs still borrow the ExecuTorch buffer, with no copy. The new test lowers one module, calls it from two places with different output sizes on one shared handle, and runs the program twice.
FoldConstantCumsumPass only folded cumsums with an int64 result. OpenVINO's CPU plugin gives the same wrong results from the second inference on for an int32 cumsum over a constant, for example cumsum(mask, dtype=int32), so such a model still got wrong attention masks. The pass now folds any integer cumsum that reads only constants. Float cumsums are still left alone. The pi0-style mask test also runs with an int32 cumsum.
An output whose size can change goes to a buffer OpenVINO owns, kept for the life of the delegate. That buffer was created at the output's current size, which on the first call is its upper bound, so a large dynamic output kept a second full-size copy around even when calls produced much less. For example, dynamic float32 logits of shape [1, 2048, 128256] cost about 1 GB more. Create it empty instead, with the output's rank. OpenVINO grows it to the size it writes, as it already did when a call needed more.
On x86 CPUs OpenVINO takes the precision when the runtime loads a program, so lowering alone cannot show that an explicit inference_precision works. If the runtime dropped it, a bf16 program would quietly compute in float32 and every test would still pass. The float32 addmm test now also lowers the same 256x256 case with bf16 and checks that the result is not within 1e-4 of eager PyTorch. It runs where OpenVINO's own CPU default is not float32, like the float32 check.
FoldConstantCumsumPass had its own check for buffers the graph writes to. It only looked at positional arguments of direct users, so a write through a view, a keyword argument or inside a higher-order op would have let it fold a buffer with its initial value. Today the only write is the positional copy_ the unlifted module adds, so nothing broke yet. Use exir's _nodes_written_in_place, which constant_prop_pass already uses and which covers those cases.
CastReductionInputPass edits the graph in place and never calls ExportPass.call(), so it does not need ExportPass. Base it on PassBase, like FoldConstantCumsumPass.
The Precision docs spoke of "a program exported before this default" running "with this runtime", which stops being clear once the change is in a release. Say instead that the default applies to every program lowered without inference_precision, including ones exported with an earlier ExecuTorch release.
FoldConstantCumsumPass folds an integer cumsum over constants, because OpenVINO's CPU plugin gives wrong results for one from the second inference on. A cumsum over torch.ones_like(x) or zeros_like(x) reads the input, so it was not folded. But with a static shape OpenVINO folds the shape into a constant, and the cumsum then hits the same bug: masks built from it were wrong on a second call with more valid tokens. ones_like and zeros_like become full_like in the edge graph, which reads only the shape and dtype of its input. The pass now counts full_like of a static shape as a constant, and evaluates it from that shape. The attention mask test also builds its mask with ones_like, and fails without this change.
bd59ee0 to
cc9b095
Compare
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 33m 59s —— View job This PR makes OpenVINO CPU programs compute in float32 by default, but the runtime also applies that default to x86 programs exported with earlier releases with no load-time opt-out, and none of the repo's OpenVINO export tools can pass Detailed reviewPR Review: #24294SummaryThe PR makes OpenVINO programs for CPU devices compute in float32 unless
Two major problems from the earlier review remain open: the load-time default changes the precision of already-deployed x86 programs, and the export tools can't set the option. The rest are minor. Backward Compatibility
API Design
Infrastructure
Testing
Performance
Code Quality
RecommendationNeeds Discussion: approval depends on two decisions from the OpenVINO maintainers.
The other findings are minor and can be fixed in this PR. Not done:
|
suryasidd
left a comment
There was a problem hiding this comment.
Changes look good to me.
|
@pytorchbot cherry-pick --onto release/1.6 -c critical |
1 similar comment
|
@pytorchbot cherry-pick --onto release/1.6 -c critical |
Cherry picking #24294The cherry pick PR is at #24377 and it is recommended to link a critical cherry pick PR with an issue. Details for Dev Infra teamRaised by workflow job |
…ault to float32 on CPU, in the OpenVINO backend (#24294) ## What is wrong today The OpenVINO backend has these problems: 1. A 0-d (scalar) input or output fails on every run, because OpenVINO's `ov_shape_create` refuses rank 0. 2. A float32 program does not compute in float32 on many CPUs. OpenVINO picks bfloat16 (two to three significant digits) on x86 CPUs with bfloat16 hardware and float16 on ARM CPUs, and no compile spec can change it. 3. An output whose size depends on an int input keeps its largest size. `x[:n].view(n, -1)` with x of shape (4, 8) and n = 3 returns (4, 8). 4. `cumsum` and `prod` of a bool or integer tensor are not computed in int64, as PyTorch does. A cumsum over a bool mask wraps after 255, so pi0 and pi0.5 get wrong position ids and wrong actions. ## What this change does - **0-d tensors:** a rank-0 tensor keeps its empty shape, which OpenVINO accepts. - **Precision:** when the device is `CPU`, a program computes in float32 by default, both when it is lowered and when it is loaded. A program can ask for a faster precision: ```python OpenvinoPartitioner( [CompileSpec("device", b"CPU"), CompileSpec("inference_precision", b"bf16")] ) ``` The value is `f32`, `f16` or `bf16`, checked before conversion starts. Other devices keep OpenVINO's default. On x86 CPUs OpenVINO applies the precision when the runtime loads the program, so the runtime passes it on too. - **Dynamic output sizes:** an output whose size can change goes to a buffer that OpenVINO owns, reused by every call. It starts empty and OpenVINO grows it to the largest size a call writes, so a large upper bound costs nothing until a call needs it. After inference the backend resizes the ExecuTorch output to OpenVINO's shape and copies the data in, so one delegate can also serve several calls, as programs from ExecuTorch 0.7 and earlier do. Static outputs still use the ExecuTorch buffer directly. - **Integer reductions:** the input of `cumsum` and `prod` is cast to int64 before conversion. An integer cumsum that reads only constants is computed during lowering, because OpenVINO's CPU plugin gives it wrong results from the second inference on. This includes a cumsum over `ones_like` or `zeros_like` of an input with a static shape, which OpenVINO folds into a constant too. Each one is folded on its own, and one that cannot be folded is named in a warning. Trade-offs: - float32 is slower. On a Xeon with bfloat16 hardware, a 4-layer MLP takes 2.4 ms in float32 and 1.5 ms in bfloat16. On x86 this also applies to programs exported before this change, because OpenVINO applies the precision when a program loads. To get the old speed, export again with `inference_precision` set to `bf16`. - OpenVINO's CPU plugin computes int64 in 32 bits, so an integer result past the int32 range wraps without an error. That was already true for int64 programs, and now also applies to `cumsum` and `prod` over smaller integer types. ## Tests Each new test fails without its fix: 0-d inputs and outputs; a 256 x 256 addmm checked to 1e-4, and the same addmm lowered with `bf16` checked not to reach 1e-4, so the runtime must apply an explicit precision (both skipped where OpenVINO already computes in float32); an unknown precision refused before conversion; dynamic output sizes, on one program and on one delegate shared by two calls; bool and int32 cumsum and prod; pi0-style masks with int64 and int32 cumsums over two calls, built from a constant and from `ones_like`; and folding when one cumsum cannot be evaluated. Two guards pass either way: integer `sum`, which OpenVINO already computes in int64, and a static output after a dynamic view. The full OpenVINO op suite passes on Linux x86_64 (a CPU with bfloat16 units) with OpenVINO 2026.4.1, through both pytest and the CI test runner. With OpenVINO 2025.1.0 it also passes, except that the prod test is skipped because that release does not delegate prod. (cherry picked from commit 7b14148)
The pick was made on release/1.6, which does not have #24298 (the #24222 pick) yet. #24294 builds on #24222, so the pick depends on code that release/1.6 lacks, such as _nodes_written_in_place in exir's constant_prop_pass, and its conflict resolution brought in some of #24222's OpenVINO changes but not others. This merges a clean pick of 7b14148 onto #24298's branch and takes the clean pick's side in the OpenVINO files, so that, on top of #24298, the change is exactly #24294 (7b14148). A plain merge of the two would keep two copies of _PRECISIONS in preprocess.py.
This pick is stacked on #24298 (the #24222 pick), which has since gained follow-up commits for the Qualcomm partitioner and the TensorRT guide. Taking them keeps this branch equal to #24298 plus #24294, so that refreshing it from release/1.6 after #24298 merges cannot undo them. The merge is clean, and the follow-ups touch no OpenVINO files.
What is wrong today
The OpenVINO backend has these problems:
ov_shape_createrefuses rank 0.x[:n].view(n, -1)with x of shape (4, 8) and n = 3 returns (4, 8).cumsumandprodof a bool or integer tensor are not computed in int64, as PyTorch does. A cumsum over a bool mask wraps after 255, so pi0 and pi0.5 get wrong position ids and wrong actions.What this change does
0-d tensors: a rank-0 tensor keeps its empty shape, which OpenVINO accepts.
Precision: when the device is
CPU, a program computes in float32 by default, both when it is lowered and when it is loaded. A program can ask for a faster precision:The value is
f32,f16orbf16, checked before conversion starts. Other devices keep OpenVINO's default. On x86 CPUs OpenVINO applies the precision when the runtime loads the program, so the runtime passes it on too.Dynamic output sizes: an output whose size can change goes to a buffer that OpenVINO owns, reused by every call. It starts empty and OpenVINO grows it to the largest size a call writes, so a large upper bound costs nothing until a call needs it. After inference the backend resizes the ExecuTorch output to OpenVINO's shape and copies the data in, so one delegate can also serve several calls, as programs from ExecuTorch 0.7 and earlier do. Static outputs still use the ExecuTorch buffer directly.
Integer reductions: the input of
cumsumandprodis cast to int64 before conversion. An integer cumsum that reads only constants is computed during lowering, because OpenVINO's CPU plugin gives it wrong results from the second inference on. This includes a cumsum overones_likeorzeros_likeof an input with a static shape, which OpenVINO folds into a constant too. Each one is folded on its own, and one that cannot be folded is named in a warning.Trade-offs:
inference_precisionset tobf16.cumsumandprodover smaller integer types.Tests
Each new test fails without its fix: 0-d inputs and outputs; a 256 x 256 addmm checked to 1e-4, and the same addmm lowered with
bf16checked not to reach 1e-4, so the runtime must apply an explicit precision (both skipped where OpenVINO already computes in float32); an unknown precision refused before conversion; dynamic output sizes, on one program and on one delegate shared by two calls; bool and int32 cumsum and prod; pi0-style masks with int64 and int32 cumsums over two calls, built from a constant and fromones_like; and folding when one cumsum cannot be evaluated. Two guards pass either way: integersum, which OpenVINO already computes in int64, and a static output after a dynamic view.The full OpenVINO op suite passes on Linux x86_64 (a CPU with bfloat16 units) with OpenVINO 2026.4.1, through both pytest and the CI test runner. With OpenVINO 2025.1.0 it also passes, except that the prod test is skipped because that release does not delegate prod.