microsoft / microsoft/onnxruntime

Where returns +0.0 for a selected -0.0, on both the X and Y branches (CPU EP)

Open
#32,191 0 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.