onnx / keras-onnx

Convert tf.keras/Keras models to ONNX
Apache License 2.0
379 stars 110 forks source link

Bug in Conv1D conversion when padding type is same #711

Open sonu1-p opened 3 years ago

sonu1-p commented 3 years ago

Hi Team, I was trying to convert a model with conv1d and padding type "same" when I countered below error:

Traceback (most recent call last):
  File "test_model.py", line 11, in <module>
    onnx_model = keras2onnx.convert_keras(model)
  File "/usr/local/lib/python3.7/site-packages/keras2onnx/main.py", line 102, in convert_keras
    return convert_topology(topology, name, doc_string, target_opset, channel_first_inputs)
  File "/usr/local/lib/python3.7/site-packages/keras2onnx/topology.py", line 309, in convert_topology
    cvt(scope, operator, container)
  File "/usr/local/lib/python3.7/site-packages/keras2onnx/ke2onnx/conv.py", line 226, in convert_keras_conv1d
    convert_keras_conv_core(scope, operator, container, is_transpose, n_dims, input_perm, output_perm, weight_perm)
  File "/usr/local/lib/python3.7/site-packages/keras2onnx/ke2onnx/conv.py", line 175, in convert_keras_conv_core
    len(input_shape))) if channels_first else input_perm_axes)
  File "/usr/local/lib/python3.7/site-packages/keras2onnx/ke2onnx/conv.py", line 40, in _calc_explicit_padding
    print((input_size[i] - 1))

Changing to

 if count_dynamic_dim(input_shape) > 0: # instead of 1 

in below line fixes the issue: https://github.com/onnx/keras-onnx/blob/383e431583d9a67da67d5a01f401b9f4df26ae4a/keras2onnx/ke2onnx/conv.py#L146

Here is a small code snippet, that demonstrates the problem:

import tensorflow as tf
import keras2onnx
import onnx

x = tf.keras.Input(shape=(None, 4), batch_size=1, name="input") # input of shape (n,d,c)
y = tf.keras.layers.Conv1D(32, 3, strides=1, activation='relu', padding="same", name="output")(x)

model = tf.keras.Model(inputs=[x], outputs=[y])
model.summary()
model.save("just_conv.h5")
onnx_model = keras2onnx.convert_keras(model)
onnx.save(onnx_model,  "just_conv.onnx")

Is it deliberate to keep count_dynamic_dim(input_shape) > 1 or this is a bug?

jiafatom commented 3 years ago

Thanks for pointing this out! I feel that count_dynamic_dim(input_shape) > 1 is for Conv2D case, because the first dim of input_shape is always None (it is a batch dim). Looks Conv1D should use 0 here, not 1.