Skip to content

[CPU] Reduce node supports fp16 precision #18227

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
Merged
dmitry-gorokhov merged 5 commits into from
Jul 18, 2023
Merged

[CPU] Reduce node supports fp16 precision #18227

dmitry-gorokhov merged 5 commits into from
Jul 18, 2023

Conversation

@xuchen-intel
Copy link
Contributor

@xuchen-intel xuchen-intel commented Jun 25, 2023

Details:

  • Reduce node supports fp16 precision.

Tickets:

@xuchen-intel xuchen-intel added the category: CPU OpenVINO CPU plugin label Jun 25, 2023
@xuchen-intel xuchen-intel requested review from a team as code owners June 25, 2023 04:39
@xuchen-intel
Copy link
Contributor Author

xuchen-intel commented Jun 25, 2023

@dmitry-gorokhov Hi Dmitry, could you please take a look?

@yuxu42
Copy link
Contributor

yuxu42 commented Jun 25, 2023

@chenhu-wang @maxnick Could you please take a review? Thanks!

@yuxu42 yuxu42 added this to the 2023.1 milestone Jun 25, 2023
@xuchen-intel xuchen-intel force-pushed the feature/reduce_node_fp16 branch 2 times, most recently from 4c6b4c5 to 9427afb Compare June 26, 2023 02:14
@xuchen-intel xuchen-intel requested a review from a team as a code owner June 26, 2023 02:14
@github-actions github-actions bot added the category: inference OpenVINO Runtime library - Inference label Jun 26, 2023
@maxnick maxnick requested a review from chenhu-wang June 26, 2023 14:08
chenhu-wang
chenhu-wang requested changes Jun 29, 2023
View reviewed changes
chenhu-wang
chenhu-wang requested changes Jun 29, 2023
View reviewed changes
src/plugins/intel_cpu/tests/functional/shared_tests_instances/skip_tests_config.cpp Outdated
Comment on lines 249 to 253
if (!InferenceEngine::with_cpu_x86_fp16()) {
// Skip fp16 tests for paltforms that don't support fp16 precision
retVector.emplace_back(R"(.*INFERENCE_PRECISION_HINT=(F|f)16.*)");
}
Copy link
Contributor

@chenhu-wang chenhu-wang Jun 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

You add with_cpu_x86_fp16() to help tell capability of avx512_f16 and use it.
https://github.com/openvinotoolkit/openvino/pull/18227/files#diff-2ec2dbc37b12dc344e3d56d476bebb0a795777886906b5598ec2c91873404c0dR76

  1. There is no limitation to support f16 only on avx512_f16. If on avx2, can support it directly. If no avx2, should be supported with fallback on f32 internally.
  2. The function name is ambiguous, maybe we can remove it as no needed.

Copy link
Contributor Author

@xuchen-intel xuchen-intel Jul 5, 2023

Choose a reason for hiding this comment

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

Agreed and applied. Thanks Chenhu! And to make this happen, this config here is also revised to be avx2 for fp16. Please feel free to commet, if there should be any concerns.
cc @dmitry-gorokhov @usstq

Copy link
Contributor

@chenhu-wang chenhu-wang Jul 8, 2023

Choose a reason for hiding this comment

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

For the change in common part, should we have performance test? As f16 is not always outperform f32, reduce maybe ok as the computing complexity is low, but not sure for other computing bound node and shapes as precision convert overhead introduced. Given not all nodes support f16, so f16 hint is actually mixed precision in execution.

Copy link
Contributor Author

@xuchen-intel xuchen-intel Jul 10, 2023

Choose a reason for hiding this comment

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

According to our discussion, for now we only activate FP16 test cases for platforms with tAVX512_FP16 instructions. FP16 test cases for AVX2 will be activated after the follow-ups in #16500 (comment) be applied. Thanks Chenhu!

@github-actions github-actions bot added category: MO Model Optimizer category: Core OpenVINO Core (aka ngraph) category: IE Tests OpenVINO Test: plugins and common category: GPU OpenVINO GPU plugin category: GNA OpenVINO GNA plugin category: build OpenVINO cmake script / infra category: Python API OpenVINO Python bindings category: transformations OpenVINO Runtime library - Transformations category: LP transformations OpenVINO Low Precision transformations category: samples OpenVINO Runtime Samples category: IR FE OpenVINO IR v10 / v11 FrontEnd category: CI OpenVINO public CI category: docs OpenVINO documentation category: tools OpenVINO C++ / Python tools category: TEMPLATE OpenVINO Template plugin category: ONNX FE OpenVINO ONNX FrontEnd labels Jul 5, 2023
@xuchen-intel xuchen-intel requested review from dmitry-gorokhov and removed request for a team July 11, 2023 01:24
dmitry-gorokhov
dmitry-gorokhov reviewed Jul 13, 2023
View reviewed changes
@xuchen-intel xuchen-intel force-pushed the feature/reduce_node_fp16 branch from 58ae9cc to d16b8b4 Compare July 14, 2023 03:36
@xuchen-intel xuchen-intel force-pushed the feature/reduce_node_fp16 branch 2 times, most recently from 7eec8d5 to bd90799 Compare July 18, 2023 07:19
@dmitry-gorokhov dmitry-gorokhov enabled auto-merge (squash) July 18, 2023 09:11
@dmitry-gorokhov dmitry-gorokhov merged commit 9334ad0 into openvinotoolkit:master Jul 18, 2023
alvoron pushed a commit to alvoron/openvino that referenced this pull request Jul 24, 2023
@xuchen-intel @alvoron
[CPU] Reduce node supports fp16 precision (openvinotoolkit#18227)
4ef5240
@dmitry-gorokhov dmitry-gorokhov mentioned this pull request Jul 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chenhu-wang chenhu-wang chenhu-wang approved these changes

@AlexKoff88 AlexKoff88 Awaiting requested review from AlexKoff88 AlexKoff88 was automatically assigned from openvinotoolkit/openvino-configuration-mgmt

@artanokhov artanokhov Awaiting requested review from artanokhov artanokhov was automatically assigned from openvinotoolkit/openvino-configuration-mgmt

@zKulesza zKulesza Awaiting requested review from zKulesza zKulesza was automatically assigned from openvinotoolkit/openvino-docs-maintainers

@slyalin slyalin Awaiting requested review from slyalin slyalin was automatically assigned from openvinotoolkit/openvino-pytorch-frontend-maintainers

@ilya-lavrenov ilya-lavrenov Awaiting requested review from ilya-lavrenov ilya-lavrenov was automatically assigned from openvinotoolkit/openvino-maintainers

@ilyachur ilyachur Awaiting requested review from ilyachur ilyachur was automatically assigned from openvinotoolkit/openvino-maintainers

@tomdol tomdol Awaiting requested review from tomdol tomdol was automatically assigned from openvinotoolkit/openvino-maintainers

+1 more reviewer

@dmitry-gorokhov dmitry-gorokhov dmitry-gorokhov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dmitry-gorokhov dmitry-gorokhov

Labels

category: CPU OpenVINO CPU plugin category: inference OpenVINO Runtime library - Inference

Projects

None yet

Milestone

2023.1

Development

Successfully merging this pull request may close these issues.

4 participants

@xuchen-intel @yuxu42 @dmitry-gorokhov @chenhu-wang