openvino icon indicating copy to clipboard operation
openvino copied to clipboard

[CORE] Test opset1::Concat shape inference with op reuse

Open barnasm1 opened this issue 9 months ago • 3 comments

Details:

  • Add new tests to cover case with reuse of pre-created operator

Tickets:

barnasm1 avatar May 06 '24 14:05 barnasm1

@jane-intel could check if anything from ticket CVS-94507 is missing. I think current shape inference works correctly and recalculate concatenation axis on each use. Added test to verify it.

praasz avatar May 07 '24 07:05 praasz

@praasz the problem was about this: https://github.com/openvinotoolkit/openvino/blob/master/src/core/src/op/concat.cpp#L45 https://github.com/openvinotoolkit/openvino/blob/master/src/core/include/openvino/op/concat.hpp#L43 https://github.com/openvinotoolkit/openvino/blob/master/src/core/include/openvino/op/concat.hpp#L60:L63

We probably need to remove the normalized axis from the node and return the newly normalized axis on each call of get_concatenation_axis set_concatenation_axis to be deleted

Alternatively, we could delete get and set concatenation_axis entirely (after deprecation). The project uses it only 14 times. image

jane-intel avatar May 13 '24 07:05 jane-intel

@praasz the problem was about this: https://github.com/openvinotoolkit/openvino/blob/master/src/core/src/op/concat.cpp#L45 https://github.com/openvinotoolkit/openvino/blob/master/src/core/include/openvino/op/concat.hpp#L43 https://github.com/openvinotoolkit/openvino/blob/master/src/core/include/openvino/op/concat.hpp#L60:L63 ...

Sure we can review if setter/getter could be marked as deprecated, I assume deprecation message should be like removed in 2025.0? The use of this concat axis could be reduced as ov::Shape and ov::PartialShape support negative index to get dimensions.

praasz avatar May 13 '24 08:05 praasz