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

Concat Op does not properly check types, resulting in redundant Cast Ops #929

Open
nolangroves opened this issue Oct 28, 2022 · 1 comment

Comments

@nolangroves
Copy link

exptype = operator.outputs[0].type
new_inputs = []
for inp in operator.inputs:
if inp.type == exptype:
new_inputs.append(inp.full_name)
continue
name = scope.get_unique_variable_name("{}_cast".format(inp.full_name))
res = exptype.to_onnx_type()
et = res.tensor_type.elem_type
apply_cast(scope, inp.full_name, name, container, to=et)
new_inputs.append(name)

The Concat operation does not properly compare typings (line 15), resulting in an excess of redundant Cast operations.
It will return False when the input and output tensors are the same type but of different sizes (which is most of the use cases of the Concat operation)

For example,
Int64TensorType(shape=[None, 1]) == Int64TensorType(shape=[None, 10])
returns False, resulting in an extra cast operation on each of the 10 Int64 inputs.

@xadupre
Copy link
Collaborator

xadupre commented Nov 10, 2022

Thanks for finding it.

xadupre added a commit to xadupre/sklearn-onnx that referenced this issue Nov 10, 2022
Signed-off-by: xadupre <xadupre@microsoft.com>
xiaowuhu pushed a commit that referenced this issue Nov 11, 2022
Signed-off-by: xadupre <xadupre@microsoft.com>

Signed-off-by: xadupre <xadupre@microsoft.com>
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

No branches or pull requests

2 participants