If-operator has unexpected semantics if condition input has anything but a single element

[cbourjau]

Describe the issue

The implementation of the If operator has surprising semantics if the condition is either empty or a tensor with more than one element. I would expect a crash or at least a warning in either case. However, the current behavior is that an empty condition evaluates to false. If a condition tensor with more than one element is provided only the first element is used.

To reproduce

Run the attached onnx model with different 1D boolean inputs.
if.onnx.gz

Urgency

No response

Platform

Mac

OS Version

12.3.1

ONNX Runtime Installation

Released Package

ONNX Runtime Version or Commit ID

1.16.1

ONNX Runtime API

Python

Architecture

ARM64

Execution Provider

Default CPU

Execution Provider Library Version

No response

[hariharans29]

But doesn't the ONNX spec explicitly state that the condition input MUST contain only a single element (https://github.com/onnx/onnx/blob/main/docs/Operators.md#If) ? having said that, maybe we could add a check here to ensure that there really is only one element -

onnxruntime/onnxruntime/core/providers/cpu/controlflow/if.cc

Line 251 in a2fd8a6

auto condition = *ctx->Input<Tensor>(0)->Data<bool>();

[yuslepukhin]

The model is invalid, and we need to detect this.

[hariharans29]

The model is invalid, and we need to detect this.

The model isn't invalid - it is valid. The model has a dynamic shape graph input of type boolean and 1-D tensor shape [N] which is then fed as the input to the If operator. So far, so good. The input sent in is invalid in the context of it being used in the If operator iff N > 1. The input sent in SHOULD be a 1-D tensor of shape [1] (N should be 1). To detect invalid inputs to the If operator, the above proposed check may be added.

[cbourjau]

Arguable, the standard should be changed such that the condition of the If node must be a scalar, but for the time being @hariharans29 's assessment is on point.

[hariharans29]

'Arguable, the standard should be changed such that the condition of the If node must be a scalar,'

Oh but doesn't it already do that? It says "Condition for the if. The tensor must contain a single element."

[cbourjau]

Oh but doesn't it already do that? It says "Condition for the if. The tensor must contain a single element."

Yes, but it should instead constrain the rank of the tensor to be rank-0 aka a scalar.

[yuslepukhin]

@cbourjau Please, close this ticket. If you feel standard chance is needed, open one at ONNX. Thx!

[cbourjau]

Thanks! Closing as fixed with #18733