PINTO0309 / onnx2tf

Self-Created Tools to convert ONNX files (NCHW) to TensorFlow/TFLite/Keras format (NHWC). The purpose of this tool is to solve the massive Transpose extrapolation problem in onnx-tensorflow (onnx-tf). I don't need a Star, but give me a pull request.
MIT License
708 stars 73 forks source link

Gather - keep output dimensions #597

Closed svobora closed 7 months ago

svobora commented 7 months ago

ONNX Gather changes output shape by removing the singled out dimension. When converting to TF using strided_slice, the extra dimension is kept. Fix to remove the extra dimension using reshape.

Example: ONNX: [1,50,768]->Gather(index=0)->[1,768] TF: [1,50,768]->StridedSlice(...)->[1,1,768]

3. Before/After (If there is an operating log that can be used as a reference)

image

PINTO0309 commented 7 months ago

Thanks.

Automated testing by CI failed. The cause is your code fails when the model contains redundant Gather and Unsqueeze in the following pattern. onnx2tf strives to optimize by automatically removing meaningless Gather and Unsqueeze combinations and replacing them with StridedSlice.

It appears that the fix you suggested will only work correctly if Unsqueeze is not present next to Gather.

CI encountered an error with the following ONNX file conversion operation. In the case of such a pattern, dimensional compression by Gather (StridedSlice + Reshape) should not be performed.

iat_llie_180x320.onnx.zip

onnx2tf -i iat_llie_180x320.onnx

image

Perhaps you can achieve the conversion behavior you expect by simply modifying the If statement as follows

    elif unsqueeze_count > 0 \
        and unsqueeze_count == consumer_count \
        and before_cast_indices is not None:

You suggested adding logic to check the shape, but I have a feeling that the conversion could probably be done without doing that, just by modifying the preceding If statement.

However, your pull request does not include some of the onnx files you used to test, so we are unable to test the conversion behavior you wish to achieve. If possible, could you please cut out a portion of the onnx file you are using for testing and attach it to this pull request.

If sharing the entire model is problematic due to project constraints, it is possible to cut out only a small portion of the ONNX file using the tools below.

https://github.com/PINTO0309/sne4onnx

svobora commented 7 months ago

Thanks for the reply, I share here onnx model, tflite model, and python code to produce the issue.

model.zip

The net is little bit different, but I believe you see the issue with the extra dimension of size 1 after conversion.

image

import torch
import torch.nn as nn
import onnx2tf

class MyModel(nn.Module):
    def __init__(self):
        super(MyModel, self).__init__()

    def forward(self, x):
        x = x[:, 0]
        print(x.shape)
        return x

model = MyModel()
onnx_input = torch.rand((1, 50, 768))

torch.onnx.export(model, onnx_input, "model.onnx", export_params=True,
                  do_constant_folding=True,
                  input_names = ["input"],
                  output_names = ["output"],
                  training=torch.onnx.TrainingMode.EVAL)

onnx2tf.convert(
    output_signaturedefs=True,
    input_onnx_file_path="model.onnx",
    output_folder_path="tmp/converted_model",
    overwrite_input_shape=["input:" + ",".join([str(x) for x in onnx_input.shape])],
    non_verbose=True,
)
svobora commented 7 months ago

Perhaps you can achieve the conversion behavior you expect by simply modifying the If statement as follows " elif unsqueeze_count > 0 \ and unsqueeze_count == consumer_count \ and before_cast_indices is not None: "

Yes, this also achieves the same goal of having correct output shape for my case.

image

PINTO0309 commented 7 months ago

Thank you.

If possible, could you try to modify the commit to only change the If statement? When you modify and push your commits, CI will automatically test regression with automated tests, so you can automatically test all edge cases except your model. It would be very helpful if you could do so, as there are hundreds of patterns I have to test in modifying the code.

image

PINTO0309 commented 7 months ago

LGTM