ThanatosShinji / onnx-tool

A parser, editor and profiler tool for ONNX models.

https://pypi.org/project/onnx-tool/

MIT License

399 stars 52 forks source link

Fix the shape inference of GridSample when the input is initializer #88

Closed ice-tong closed 3 months ago

ice-tong commented 3 months ago

Motivtion

The shape inference of GridSampleNode could be crash if the input is initializer.

Traceback (most recent call last):
  ...
  File "/home/xxx/miniconda3/envs/torch112cu113/lib/python3.10/site-packages/onnx_tool/__init__.py", line 84, in model_profile
    g.shape_infer(dynamic_shapes)
  File "/home/xxx/miniconda3/envs/torch112cu113/lib/python3.10/site-packages/onnx_tool/graph.py", line 830, in shape_infer
    node.shape_infer(itensors, otensors)
  File "/home/xxx/miniconda3/envs/torch112cu113/lib/python3.10/site-packages/onnx_tool/node.py", line 818, in shape_infer
    out_shape = intensors[0].shape[:2] + intensors[1].shape[1:1+r]
TypeError: can only concatenate list (not "tuple") to list

The intensors[1].shape would be a tuple if it is a initializer tensor.

Modification

onnx_tool/node.py@GridSampleNode
- Handle the case that input tensor shape is tuple in the shape_infer.

ThanatosShinji commented 3 months ago

would you mind sharing the model that the input of this node is an initializer? I may consider keeping all tensor shapes as a List variable.

ice-tong commented 3 months ago

would you mind sharing the model that the input of this node is an initializer? I may consider keeping all tensor shapes as a List variable.

Hi @ThanatosShinji , thanks for your prompt reply!

I'm sorry I can't share this model with you. It is a BEVFormer model based on this repository, modified and exported: https://github.com/DerryHub/BEVFormer_tensorrt.

The following screenshot is an example of gridsample input with constants, hope it helps.

ice-tong commented 3 months ago

would you mind sharing the model that the input of this node is an initializer? I may consider keeping all tensor shapes as a List variable.

If you prefer to unify Tensor.shape as a list type, I can modify this PR.

Just convert self.numpy.shape to list by modifying the following code. I'm not sure if this change will cause any other problems though.

https://github.com/ThanatosShinji/onnx-tool/blob/c17ef5b5200fc95fd15975af844283a1c1e4e2ea/onnx_tool/tensor.py#L380-L388

ThanatosShinji commented 3 months ago

would you mind sharing the model that the input of this node is an initializer? I may consider keeping all tensor shapes as a List variable.

If you prefer to unify Tensor.shape as a list type, I can modify this PR.

Just convert self.numpy.shape to list by modifying the following code. I'm not sure if this change will cause any other problems though.

https://github.com/ThanatosShinji/onnx-tool/blob/c17ef5b5200fc95fd15975af844283a1c1e4e2ea/onnx_tool/tensor.py#L380-L388

Sure, please add the List conversion here.

ice-tong commented 3 months ago

Hi @ThanatosShinji, sorry for the late update. I just pushed a new commit to convert the shape to a list for the onnx.TensorProto case. Please check it out and feel free to leave your comments!

ThanatosShinji commented 3 months ago

Thanks @ice-tong !