KhronosGroup / NNEF-Tools

The NNEF Tools repository contains tools to generate and consume NNEF documents
https://www.khronos.org/nnef
222 stars 58 forks source link

Tensor Shapes in ONNX to NNEF in Latest Commits #145

Closed adnn261 closed 3 years ago

adnn261 commented 3 years ago

After getting the latest commits, I now get an error such as:

Input graph contains tensors with unknown shape: '101', '106',...

when trying to convert an onnx model to nnef. What has happened with shape inference?

gyenesvi commented 3 years ago

Did this conversion work before? When was the last time you tried it? Shape inference is still there, but at some point, some extra checks were added because even after shape inference some shapes could remain unknown, that could crash the converter.

Please check the following:

If these don't help, can you tell which is the operator in your model that produces an output tensor of unknown shape, probably tensor 101, or one of those in the beginning of the list.

adnn261 commented 3 years ago

Thank you for your response. I did not have --fold-constants on, but turning it on does not seem to be fixing the problem. I'm not using any dynamic input shapes as far as I know.

The operator that produces tensor '101' the "split" function. Errors for other tensors seem to follow from operators that are dependent on '101'.

gyenesvi commented 3 years ago

The split operator has a second input argument that determines the split sizes. If that is a non-constant tensor, then the outputs' shapes are not known. Is that the case, or is the second argument to split a constant (or missing in case of equal splits)?

Is your model confidential or can you share it so that I can debug faster?

adnn261 commented 3 years ago

Here is an example where the error occurs:

split_test.zip

gyenesvi commented 3 years ago

Thanks for the example, I have checked it and what happens is that if I run onnx.shape_inference.infer_shapes on this model, it does not infer the output shapes of the split operation (the shape is left undefined, and the data-type is inferred correctly). Not sure why that happens, or is that something that has changed recently in ONNX shape inference.

Anyway, the ONNX to NNEF conversion simply relies on those shapes being available, so it seems that there is no bug in the NNEF converter, but this should be fixed in the onnx function. It would be interesting ti know if that's a bug or a feature.

gyenesvi commented 3 years ago

Okay, this seems to be a known issue with onnx shape inference for almost two years, they thought they have fixed it, but apparently not as of 23 Sep:

https://github.com/onnx/onnx/issues/1735

adnn261 commented 3 years ago

Thank you so much for your response! This error was occuring before, but only after the recent (major) updates to NNEF Tools.

Is this because the newest NNEF tools became more dependent on ONNX and other external libraries for shape inferencing, compared to before?

gyenesvi commented 3 years ago

Yes, you are right, in the previous version we wrote more and more shape inference code for all the frameworks as it was needed more and more for the conversion, and we realized that we are duplicating a lot of functionality that is already present in the frameworks, so we decided to just use what the frameworks offer; it's more up-to-date, less error-prone and less of a maintenance burden. Of course, it has the downside that if something is wrong in the frameworks then it propagates to our tools. Not sure if this could be worked around somehow.

adnn261 commented 3 years ago

I see. Thank you so much for looking after this!

gyenesvi commented 3 years ago

Closing this as there is nothing to do on our side.