Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw for scalar input to Shape. #18504

Closed
wants to merge 1 commit into from

Conversation

skottmckay
Copy link
Contributor

Description

Shape returns an empty tensor with shape {0} for a scalar, which is incorrect as there is a single data element. Throwing will at least make the cause of the failure clear.

Expected might be for it to return a shape of {}, but ONNX shape inferencing doesn't support scalar either as it calls add_dim on the output_shape at the start so result will always be rank 1 or higher.

https://github.com/onnx/onnx/blob/b60f69412abb5393ab819b936b473f83867f6c87/onnx/defs/tensor/defs.cc#L496

The ONNX spec doesn't say scalars aren't supported. @gramalingam should scalar input to Shape return an empty shape?

Motivation and Context

Model failed downstream due to nullptr in output from Shape when called with a scalar.

…correct as there is a single data element.

Expected might be for it to return a shape of `{}`, but ONNX shape inferencing doesn't support scalar either as it calls add_dim on the output_shape at the start so result will always be rank 1 or higher.

https://github.com/onnx/onnx/blob/b60f69412abb5393ab819b936b473f83867f6c87/onnx/defs/tensor/defs.cc#L496

The spec doesn't say scalars aren't supported.
@gramalingam
Copy link
Contributor

The shape of a scalar is {}. The question is its rank. It makes sense that Shape returns a 1-dimensional tensor with k elements, where k could be zero as a special case. But, {0} is incorrect (for a scalar input). (Though it is valid return value for a 1-dimensional tensor with zero elements.) So, the onnx shape inference seems correct to me.

I don't understand your concern about "so result will always be rank 1 or higher". It will be rank 1, but with zero elements, and that is fine?

@skottmckay
Copy link
Contributor Author

Sorry - I misinterpreted what the shape inferencing was doing due to misconfiguring my unit test and confused myself between empty shapes and shapes for input with no data.

I think after all that it's expected to return nullptr in this case as there is an empty shape for a scalar - so there's no shape info to return.

Slightly confusingly (to me anyway) for an input with no actual data, using input with a shape of {0} as a simple example, it will return a 1D tensor a shape of {1} that has a value of 0 in it.

So we get nullptr output for a scalar input, and 1D output for an input which had a shape but no data due to a dim having a value of 0.

@skottmckay skottmckay closed this Nov 20, 2023
@gramalingam
Copy link
Contributor

I still don't understand the root-cause for the model that failed downstream. Is that resolved or not? I don't understand the nullptr issue. I think that onnxruntime needs to be able to handle the special case of tensors that have size 0, but a very specific shape: for example a tensor of shape 128 x 0 x 256, say. A tensor-representation combines shape information ({120, 0, 256} in this example, with data information (no data in this example). It seems fine if the dataptr is null in this special case, but the tensor-pointer will be non-null.

Is my understanding correct? So, is the problem that something downstream assumes dataptr is non-null, when it is null?

@skottmckay skottmckay deleted the skottmckay/ValidateShapeInput branch September 5, 2024 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants