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.

Sign up for GitHub

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

Jump to bottom

single-image-test: added accuracy workaround for computing CPU reference and adjust the NRMSE and PSNR metrics #26020

Merged
merged 3 commits into from
Oct 2, 2024
Merged

single-image-test: added accuracy workaround for computing CPU reference and adjust the NRMSE and PSNR metrics #26020

merged 3 commits into from
Oct 2, 2024

Conversation

beleiuandrei
Copy link
Contributor

@beleiuandrei beleiuandrei commented Aug 12, 2024 •
edited
Loading

Details:

Small improvements for SIT:

  • Disable dynamic quantization feature, which is used by default by CPU pipeline to generate the reference outputs and affects accuracy for some particular models
  • Increase number of decimals for reported NRMSE metric to improve
  • For PSNR metric report fails only when the accuracy decrease not when it increase above the target

Tickets:

@github-actions github-actions bot added the category: NPU OpenVINO NPU plugin label Aug 12, 2024
@beleiuandrei beleiuandrei changed the title single-image-test: set DYNAMIC_QUANTIZATION_GROUP_SIZE to 0 for CPU single-image-test: disable dynamic quantization feature for CPU Aug 12, 2024
@beleiuandrei beleiuandrei marked this pull request as ready for review August 12, 2024 08:11
@beleiuandrei beleiuandrei requested review from a team as code owners August 12, 2024 08:11
@pereanub
Copy link
Contributor

build_jenkins

2 similar comments
@beleiuandrei
Copy link
Contributor Author

build_jenkins

@beleiuandrei
Copy link
Contributor Author

build_jenkins

@beleiuandrei
Copy link
Contributor Author

build_jenkins

@beleiuandrei
Copy link
Contributor Author

@PatrikStepan @pereanub can you please have a look? Thank you!

@beleiuandrei
Copy link
Contributor Author

build_jenkins

@beleiuandrei
Copy link
Contributor Author

build_jenkins

1 similar comment
@beleiuandrei
Copy link
Contributor Author

build_jenkins

@pereanub
Copy link
Contributor

@Maxim-Doronin could you have a look please?

Maxim-Doronin
Maxim-Doronin reviewed Sep 23, 2024
View reviewed changes
src/plugins/intel_npu/tools/single-image-test/main.cpp
@@ -1302,7 +1304,7 @@ bool testPSNR(const TensorMap& outputs, const TensorMap& references, const int d

auto result = utils::runPSNRMetric(actOutput, refOutput, dstHeight, dstWidth, scaleBorder, normalizedImage);

if (std::fabs(result - FLAGS_psnr_reference) > FLAGS_psnr_tolerance) {
if (FLAGS_psnr_reference - result > FLAGS_psnr_tolerance) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we expect a negative value here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it can happen that PSNR metric to provide value greater then the FLAGS_psnr_reference which means that the NPU actual result beats the reference. For this metric, the larger the result is the better accuracy is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation!

@Maxim-Doronin
Copy link
Contributor

Maxim-Doronin commented Sep 23, 2024 •
edited
Loading

It's not clear from the PR description how this tiny change is related to dynamic quantization

@beleiuandrei beleiuandrei changed the title single-image-test: disable dynamic quantization feature for CPU single-image-test: added accuracy workaround for computing CPU reference and adjust the NRMSE and PSNR metrics Sep 23, 2024
@beleiuandrei
Copy link
Contributor Author

It's not clear from the PR description how this tiny change is related to dynamic quantization

I updated the PR description. We disable dynamic quantization for CPU plugin.

@beleiuandrei
Copy link
Contributor Author

build_jenkins

@pereanub pereanub added this pull request to the merge queue Oct 2, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 2, 2024
@beleiuandrei
single-image-test: added accuracy workaround for computing CPU refere…
287fb97 
…nce and adjust the NRMSE and PSNR metrics (#26020)

### Details:
Small improvements for SIT:
- Disable [dynamic quantization
feature](https://github.com/openvinotoolkit/openvino/blob/master/src/inference/include/openvino/runtime/properties.hpp#L574),
which is used by default by CPU pipeline to generate the reference
outputs and affects accuracy for some particular models
 - Increase number of decimals for reported NRMSE metric to improve
- For PSNR metric report fails only when the accuracy decrease not when
it increase above the target

### Tickets:
 - *[CVS-143420](https://jira.devtools.intel.com/browse/CVS-143420)*
@pereanub pereanub removed this pull request from the merge queue due to a manual request Oct 2, 2024
@beleiuandrei
Copy link
Contributor Author

build_jenkins

@beleiuandrei beleiuandrei added this pull request to the merge queue Oct 2, 2024
@beleiuandrei beleiuandrei removed this pull request from the merge queue due to a manual request Oct 2, 2024
@beleiuandrei beleiuandrei added this pull request to the merge queue Oct 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 2, 2024
@beleiuandrei
Copy link
Contributor Author

build_jenkins

@beleiuandrei beleiuandrei added this pull request to the merge queue Oct 2, 2024
Merged via the queue into openvinotoolkit:master with commit cddcfe8 Oct 2, 2024
130 checks passed
@beleiuandrei beleiuandrei deleted the abeleiu/sit_improvment branch October 2, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Maxim-Doronin Maxim-Doronin Maxim-Doronin approved these changes

Assignees
No one assigned
Labels
category: NPU OpenVINO NPU plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@beleiuandrei @pereanub @Maxim-Doronin