openvino icon indicating copy to clipboard operation
openvino copied to clipboard

Add ConcatToTile transformation

Open CuriousPanCake opened this issue 1 year ago • 4 comments

Details:

  • Add a ConcatToTile transformation to replace Concat having inputs from the same output with a Tile or Broadcast
  • Add a test for the ConcatToTile transformation
  • Make shared_op_optimization() really visit nodes only once and do not iterate over the same nodes several times.
  • Significantly reduce model compile time and performance time.

Tickets:

  • CVS-138829, CVS-138077

Signed-off-by: Andrii Staikov [email protected]

CuriousPanCake avatar May 20 '24 20:05 CuriousPanCake

Added Tile. Tests to be added/fixed later.

CuriousPanCake avatar May 22 '24 22:05 CuriousPanCake

Approve Tile transformation, but not the change in SharedOp. IR names would change because output choice is non deterministic. Can we keep the vector to store outputs and check if the same output exists in this vector?

It's deterministic if we are talking about running the same model twice or more. The algo will work the same way.

If we are talking about some debug capabilities. Or friendly_names get_target_inputs already returns set<Input< Node> > which will be sorted by Node index order and Node address image So the order is non deterministic already.

Maybe it's better to use set instead of unordered_set to achieve the same effect. It will use the same sorting algorithm.

itikhono avatar May 23 '24 13:05 itikhono

Concat to Tile causes an issue in e2e tests with unknown root cause. We decided to exclude Tile from transformation and leave Concat to Broadcast case only. https://github.com/openvinotoolkit/openvino/pull/24661

itikhono avatar May 23 '24 23:05 itikhono

This PR will be closed in a week because of 2 weeks of no activity.

github-actions[bot] avatar Jul 01 '24 00:07 github-actions[bot]

This PR was closed because it has been stalled for 2 week with no activity.

github-actions[bot] avatar Jul 09 '24 00:07 github-actions[bot]