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

MVN test extensions #6291

Conversation

chenhu-wang
Copy link
Contributor

@chenhu-wang chenhu-wang commented Jun 22, 2021

Details:

  • create MVN1+FQ+MVN2 subgraph test. This pattern is transformed to MVN1_fused_with_quantize + MVN2_fused_with_dequantize. MVN1_fused_with_quantize will produce and store lp data. MVN2_fused_with_dequantize will load lp data. This is to cover load/store emitters which is responsible data load/store in MVN. Meanwhile this can cover round type of precision convert in MVN_fused_with_quantize

Tickets:

  • 56733
  • 57688 test cover

@chenhu-wang chenhu-wang added category: IE Tests OpenVINO Test: plugins and common category: CPU OpenVINO CPU plugin labels Jun 22, 2021
@chenhu-wang chenhu-wang requested a review from a team June 22, 2021 08:33
@chenhu-wang chenhu-wang force-pushed the chenhu/MVN_test_extension branch from 819e13f to 62f4636 Compare June 23, 2021 07:44
@chenhu-wang chenhu-wang added this to the 2022.1 milestone Jun 23, 2021
@dmitry-gorokhov
Copy link
Contributor

@antonvor could you please review? We also need to understand do we have better way to validate MVN with fused FQ and i8/u8 output precisions.

@antonvor
Copy link
Contributor

@dmitry-gorokhov yes, the functionality here #6546 allows you to test similar cases. We can not write a separate subgraph test, but expand the existing fusing test.

Copy link
Contributor

@antonvor antonvor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I think it is possible to try to extend the existing test using mechanism in #6546. Perhaps there will be comments and we will improve it. @chenhu-wang and @dmitry-gorokhov what do you think?

@chenhu-wang
Copy link
Contributor Author

LGTM. I think it is possible to try to extend the existing test using mechanism in #6546. Perhaps there will be comments and we will improve it. @chenhu-wang and @dmitry-gorokhov what do you think?

  1. This is a subgraph(x + FQ + MVN) that reproduce and cover bug in real model.
  2. Currently MVN reference implementation do not have best accuracy for low precision data calculation. With MVN(fp32) + FQ (fp32) + MVN(fp32), ng ref can return accurate result with fp32 execution and pass the comparison, and meanwhile CPU plugin is transformed executed with low precision.
    Above is my thought to keep this test.

@dmitry-gorokhov dmitry-gorokhov modified the milestones: 2022.1, 2022.2 Feb 8, 2022
@dmitry-gorokhov dmitry-gorokhov modified the milestones: 2022.2, 2022.3 Aug 18, 2022
@dmitry-gorokhov
Copy link
Contributor

Outdated PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: CPU OpenVINO CPU plugin category: IE Tests OpenVINO Test: plugins and common
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants