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

[CPU] Fix resolve edge conflicts Convert insertion #25643

Conversation

EgorDuplensky
Copy link
Contributor

@EgorDuplensky EgorDuplensky commented Jul 19, 2024

Details

  • Current version does not clean extra edge in case Convert + Reorder is inserted. In this case we need to remove an edge, which is replaced with Convert node first, and then remove childEdge of the newly inserted Convert node, after Reorder is inserted.
  • The idea is to simplify this logic and do one thing at a time:
  1. Insert Convert.
  2. If Reorder is still necessary for the new inserted edge, it will be handled later in the loop.

It seems to be impossible to write a test for x86 platform, since there is no reorder which does not support precision conversion.

@EgorDuplensky EgorDuplensky requested review from a team as code owners July 19, 2024 13:27
@github-actions github-actions bot added the category: CPU OpenVINO CPU plugin label Jul 19, 2024
@EgorDuplensky EgorDuplensky force-pushed the refactor_resolve_edge_conflicts branch 4 times, most recently from 08f31bb to 0676929 Compare July 19, 2024 15:52
@EgorDuplensky EgorDuplensky changed the title [CPU] Simplify resolve edge conflicts Convert insertion [CPU] Fix resolve edge conflicts Convert insertion Jul 19, 2024
@dmitry-gorokhov dmitry-gorokhov added this to the 2024.4 milestone Jul 22, 2024
@dmitry-gorokhov
Copy link
Contributor

@EgorDuplensky @alvoron
What was the precision/layout configuration caused issue on ARM?
Still think it worth to implement subgraph test even if it will be benefitial only for non x86 systems.

@alvoron
Copy link
Contributor

alvoron commented Jul 22, 2024

@EgorDuplensky @alvoron What was the precision/layout configuration caused issue on ARM? Still think it worth to implement subgraph test even if it will be benefitial only for non x86 systems.

Interpolate(f32, abcd) -> Convolution(f16, acdb)

@EgorDuplensky EgorDuplensky force-pushed the refactor_resolve_edge_conflicts branch 2 times, most recently from 8bc6d1e to 4ee44d8 Compare July 22, 2024 17:46
@EgorDuplensky
Copy link
Contributor Author

logic re-written with switch-case

@EgorDuplensky EgorDuplensky force-pushed the refactor_resolve_edge_conflicts branch from 4ee44d8 to 030efd9 Compare August 14, 2024 12:49
@EgorDuplensky
Copy link
Contributor Author

rebased

@dmitry-gorokhov dmitry-gorokhov added this pull request to the merge queue Aug 16, 2024
Merged via the queue into openvinotoolkit:master with commit 8713ca2 Aug 16, 2024
132 checks passed
@dmitry-gorokhov dmitry-gorokhov deleted the refactor_resolve_edge_conflicts branch August 16, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: CPU OpenVINO CPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants