microsoft / microsoft/onnxruntime
Where returns +0.0 for a selected -0.0, on both the X and Y branches (CPU EP)
- Dominant language
- C++
- Stars
- 21.9k
- Forks
- 4.2k
- Avg merge
- 4d 11h
- Merged PRs (30d)
- 184
Description
### Describe the issue
`Where` does not preserve `-0.0`. A `-0.0` selected from `X` is returned as `+0.0`, and so is a
`-0.0` selected from `Y` when `Y` is broadcast against a wider `X`.
ONNX defines `Where` as returning elements from `X` or `Y` depending on `condition`, with
NumPy-style multidirectional broadcasting. Since this is elementwise selection rather than
arithmetic, the selected element should be preserved. `+0.0` and `-0.0` are distinct IEEE-754
values with distinct bit patterns, and `onnx.reference` preserves the negative zero in both of
the cases under "To reproduce".
## Root cause
`onnxruntime/core/providers/cpu/tensor/where_op.cc`. `Where::Compute` states the algorithm:
```cpp
// X_selection = condition ? X : default value
// Y_selection = !condition ? Y : default value
// output = (X_selection != default value) ? X_selection : Y_selection
```
For floats the default is `+0.0`, and `-0.0 == 0.0`. So a `-0.0` selected from `X` compares equal
to the default, the merge treats it as unselected, and takes `Y_selection`, which holds `+0.0` at
that position.
Two comparisons implement that check, between them covering all three merge functors of the
`EnableIfEigenScalar` overload of `MergeBroadcastFuncs` (lines 153 and 178):
```cpp
if (scalar_value != T{}) { ... } // line 153, inside MergeScalarAndVector
return x != T{} ? x : y; // line 178, both operands non-scalar
```
This is why the `Y` branch differs between the two cases. At equal shapes `Y_selection` is the
untested fall-through of the merge, so a `-0.0` there survives. When `Y` is broadcast it becomes
the scalar operand of `MergeScalarAndVector`, so it *is* the value being tested, and the sign is
lost.
The relevant `Where` implementation is identical on main, v1.29.0, and v1.28.0.
## Suggested fix
A `-0.0` can only be present in a selection tensor if it was selected, since the default has its
sign bit clear. So the test only has to admit it:
```cpp
template
constexpr bool WasSelected(const T& value) {
if constexpr (std::is_floating_point::value) {
return value != T{} || std::signbit(value);
} else {
return value != T{};
}
}
```
in place of the two `!= T{}` tests. Selecting `+0.0` from `X` still falls through to `Y_selection`,
which holds `+0.0` at that position, so nothing else changes.
For the CPU kernel, the affected registered floating-point types are `float` and `double`.
`MLFloat16` and `BFloat16` are present in the registration block but commented out. The
`std::string` specialization uses a separate overload, where the empty string is the default value
and is indistinguishable between the two selection arms, so it is not affected by this signed-zero
issue.
One testing note: `OpTester` compares floating point outputs numerically, and `-0.0f == 0.0f`, so a
signed-zero regression is invisible to it. The sign bit has to be checked explicitly through
`SetCustomOutputVerifier`, as in #31477.
Found by differential testing of single-node models against `onnx.reference` and `tract`, comparing
outputs by bit pattern rather than by `==`, which treats `-0.0` and `0.0` as equal and hides this
entirely.
### To reproduce
Needs `pip install onnx onnxruntime`, and runs as shown against onnxruntime 1.29.0 and onnx 1.22.0.
Bit patterns are printed, since `-0.0` and `0.0` print identically. Opset 22.
```python
import numpy as np, onnx, onnxruntime as ort
from onnx import TensorProto as T, helper as h
from onnx.reference import ReferenceEvaluator
def where(label, cond, x, y, out_shape):
g = h.make_graph([h.make_node("Where", ["c", "x", "y"], ["out"])], "where",
[h.make_tensor_value_info("c", T.BOOL, list(cond.shape)),
h.make_tensor_value_info("x", T.FLOAT, list(x.shape)),
h.make_tensor_value_info("y", T.FLOAT, list(y.shape))],
[h.make_tensor_value_info("out", T.FLOAT, out_shape)])
m = h.make_model(g, opset_imports=[h.make_opsetid("", 22)]); m.ir_version = 10
onnx.checker.check_model(m)
feeds = {"c": cond, "x": x, "y": y}
bits = lambda a: " ".join(f"0x{v:08x}" for v in np.ascontiguousarray(a).view(np.uint32).ravel())
print(f"{label}\n onnxruntime "
f"{bits(ort.InferenceSession(m.SerializeToString()).run(None, feeds)[0])}"
f"\n onnx.reference {bits(ReferenceEvaluator(m).run(None, feeds)[0])}")
f32 = np.float32
where("X branch, equal shapes",
np.array([True]), np.array([-0.0], f32), np.array([0.0], f32), [1])
where("Y branch, Y broadcast against a 4-wide X",
np.array([False]), np.array([1.0, 2.0, 3.0, 4.0], f32), np.array([-0.0], f32), [4])
```
Output:
```
X branch, equal shapes
onnxruntime 0x00000000 <- expected 0x80000000
onnx.reference 0x80000000
Y branch, Y broadcast against a 4-wide X
onnxruntime 0x00000000 0x00000000 0x00000000 0x00000000 <- expected 0x80000000 x4
onnx.reference 0x80000000 0x80000000 0x80000000 0x80000000
```
A `-0.0` selected from `Y` at *equal* shapes is returned correctly; the root cause explains why.
`float64` behaves the same way, and the result is unaffected by graph optimization level.
### Urgency
_No response_
### Platform
Mac
### OS Version
15.7.3
### ONNX Runtime Installation
Released Package
### ONNX Runtime Version or Commit ID
1.29.0
### ONNX Runtime API
Python
### Architecture
ARM64
### Execution Provider
Default CPU
### Execution Provider Library Version
_No response_
Contributor guide
Research direction
Start in onnxruntime/core/providers/cpu/tensor/where_op.cc, focusing on the MergeBroadcastFuncs overloads and the two comparisons identified in the issue. Review the signed-zero verifier pattern in #31477, then add coverage for both float branches and confirm the output bit patterns preserve -0.0 while existing Where behavior remains unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- backend, machine-learning
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100