not create new symbol with immutable shape convolution. #1867

[0seba]

🌱

Hi, according to this PR which looked to be merged, when a convolution does not mutate dimension size for a symbolic shape, no new symbol should be created, but this is not what I seed in the code. So should we expect this behavior or not?

Current implementation of conv shape inference

https://github.com/apple/coremltools/blob/7521b68fba363d4add0c772750d119e4d9815ce6/coremltools/converters/mil/mil/ops/defs/_utils.py#L326-L327

This is a minimal example to verify a new symbol is created

w = np.ones((128, 64, 1, 1))

@mb.program(
    input_specs=[
        mb.TensorSpec(mil.input_types.EnumeratedShapes(shapes=[(1, 64, 1, 128), (1, 64, 1, 256)]).symbolic_shape, dtype=mil.input_types.types.fp32),
    ],
    opset_version=mil.builder.AvailableTarget.iOS17,
)
def conv_shape_debug(x):
    return mb.conv(x=x, weight=w)

print(conv_shape_debug)
>>>main[CoreML7](%x: (1, 64, 1, is586, fp32)(Tensor)) {
  block165() {
    %conv_7: (1, 128, 1, is587, fp32)(Tensor) = conv(x=%x, weight=%conv_7_weight_0, strides=[1, 1], pad_type="valid", pad=[0, 0, 0, 0], dilations=[1, 1], groups=1, name="conv_7")
  } -> (%conv_7)
}

[TobyRoseman]

I'm not sure what you're asking. Can you please clarify the question?

Also, please include all code necessary to reproduce the issue.

[0seba]

Input has symbolic value is586 in the last dimension of the shape and output is587. It can be inferred from the convolution parameters that that operation does not modify the shape, so it shouldn't be necessary to create a new symbol and output can also have is586 as last dimension of the shape

[TobyRoseman]

Can you share your complete code to reproduce this issue?

[0seba]

In the example above I expect the output's spatial dimension symbol is587, to be instead the same as the input, is586. In the provided code I think only the imports would be missing

from coremltools.converters.mil import Builder as mb
import coremltools.converters.mil as mil
import numpy as np

Situations in which this symbol mismatch causes problem is if I then want to apply operations that require shape matching, like einsum.

[TobyRoseman]

Are you not calling ct.convert on conv_shape_debug?

[0seba]

It does convert correctly if I do not use operations like einsum, but fails to build the program if I do use it. For example, I could have 2 branches with convolutions that maintain the spatial shape, but since new symbols are created from them, I would not be able to use einsum between them because it requires shape matching

[TobyRoseman]

Can you share all the code we need to reproduce this issue? Including the ct.convert call and any thing else needed.