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

Enable new FP16 and support mixed precision by MO #8514

Merged
merged 21 commits into from
Dec 24, 2021
Merged

Enable new FP16 and support mixed precision by MO #8514

merged 21 commits into from
Dec 24, 2021

Conversation

mvafin
Copy link
Contributor

@mvafin mvafin commented Nov 10, 2021 •
edited
Loading

Details:

  • Enable new FP16 IR generation that was disabled in Temporarily disable new FP16 generation #8650
  • Disable converting to FP32 by default in MO which allows mixed-precision IR
  • Replace usage of np.array to mo_array which forces fp32 dtype if no dtype was passed in MO

Tickets:

  • 66048

@mvafin mvafin requested a review from a team November 10, 2021 13:32
@openvino-pushbot openvino-pushbot added the category: MO Model Optimizer label Nov 10, 2021
@jane-intel jane-intel self-assigned this Nov 18, 2021
@mvafin mvafin force-pushed the mo/mixed_precision branch 2 times, most recently from 79a3084 to d3e3ce5 Compare November 18, 2021 16:41
@mvafin mvafin requested a review from a team November 18, 2021 16:41
@mvafin mvafin requested a review from a team as a code owner November 18, 2021 16:55
@mvafin mvafin changed the title Support mixed precision by MO Enable new FP16 and support mixed precision by MO Nov 19, 2021
@nkogteva nkogteva mentioned this pull request Nov 19, 2021
Closed
@Maxim-Doronin
Copy link
Contributor

Maxim-Doronin commented Nov 19, 2021 •
edited
Loading

@vinograd47 @ArtemySkrebkov-intel @AntonDudchenko please take a closer look. It may have a critical impact on vpu functionality

@mvafin
Copy link
Contributor Author

mvafin commented Nov 19, 2021

Because of replacements on MO side this PR have a lot of changes. To better review it, please look into separate commits.

@ArtemySkrebkov
Copy link
Contributor

@olegmorenov

Can you please generate a package of IR using this PR for us to test?

@ArtemySkrebkov ArtemySkrebkov mentioned this pull request Nov 22, 2021
Closed
@mvafin mvafin added this to the 2022.1 milestone Nov 22, 2021
@mvafin mvafin mentioned this pull request Nov 30, 2021
Closed
@mvafin mvafin requested review from a team November 30, 2021 20:41
@mvafin mvafin requested a review from a team December 3, 2021 11:51
@rkazants
Copy link
Contributor

rkazants commented Dec 17, 2021 •
edited
Loading

Please fix a bug exposed in e2e. I will run one more DLB run on weekend:

[ ERROR ]  Traceback (most recent call last):
  File "/home/jenkins/agent/workspace/private-ci/ie/e2e-tests-linux-ubuntu20-e2e/b/tmp/e2e_test_venv/lib/python3.6/site-packages/openvino/tools/mo/utils/class_registration.py", line 278, in apply_transform
    for_graph_and_each_sub_graph_recursively(graph, replacer.find_and_replace_pattern)
  File "/home/jenkins/agent/workspace/private-ci/ie/e2e-tests-linux-ubuntu20-e2e/b/tmp/e2e_test_venv/lib/python3.6/site-packages/openvino/tools/mo/middle/pattern_match.py", line 46, in for_graph_and_each_sub_graph_recursively
    func(graph)
  File "/home/jenkins/agent/workspace/private-ci/ie/e2e-tests-linux-ubuntu20-e2e/b/tmp/e2e_test_venv/lib/python3.6/site-packages/openvino/tools/mo/front/tf/WhileNormalize.py", line 23, in find_and_replace_pattern
    self.normalize_loop_node(graph, node)
  File "/home/jenkins/agent/workspace/private-ci/ie/e2e-tests-linux-ubuntu20-e2e/b/tmp/e2e_test_venv/lib/python3.6/site-packages/openvino/tools/mo/front/tf/WhileNormalize.py", line 36, in normalize_loop_node
    'value': mo_array(True, dtype=np.bool)}).create_node()
NameError: name 'mo_array' is not defined

rkazants
rkazants requested changes Dec 17, 2021
View reviewed changes
Copy link
Contributor

@rkazants rkazants left a comment

Choose a reason for hiding this comment

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

Please fix failure. Also, I need to review these changes one more time.

@mvafin mvafin requested a review from rkazants December 18, 2021 09:00
rkazants
rkazants requested changes Dec 18, 2021
View reviewed changes
Copy link
Contributor

@rkazants rkazants left a comment

Choose a reason for hiding this comment

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

Disagree with these changes regarding "Replace usage of np.array to mo_array which forces fp32 dtype if no dtype was passed in MO". Please revert

rkazants
rkazants requested changes Dec 20, 2021
View reviewed changes
Copy link
Contributor

@rkazants rkazants left a comment

Choose a reason for hiding this comment

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

please apply comments

tools/mo/openvino/tools/mo/back/compress_quantized_weights.py Show resolved Hide resolved
tools/mo/openvino/tools/mo/front/InterpolateNormalizer.py Outdated Show resolved Hide resolved
tools/mo/openvino/tools/mo/front/MatMul_normalizer.py Show resolved Hide resolved
tools/mo/openvino/tools/mo/front/MatMul_normalizer.py Show resolved Hide resolved
tools/mo/openvino/tools/mo/front/common/partial_infer/utils.py Show resolved Hide resolved
tools/mo/openvino/tools/mo/pipeline/common.py Show resolved Hide resolved
tools/mo/openvino/tools/mo/main.py Outdated Show resolved Hide resolved
tools/mo/openvino/tools/mo/main.py Outdated Show resolved Hide resolved
tools/mo/openvino/tools/mo/front/tf/bucketize_ext.py Outdated Show resolved Hide resolved
tools/mo/openvino/tools/mo/front/onnx/group_norm_ext.py Show resolved Hide resolved
rkazants
rkazants reviewed Dec 22, 2021
View reviewed changes
tools/mo/openvino/tools/mo/ops/convolution.py
Comment on lines +301 to +306
if in_type_1 in [np.float16, np.float32, np.float64] and in_type_0 != in_type_1 and in_node_1.op == 'Const':
in_node_1 = node.in_port(1).get_source().node
log.error("Changing Const node '{}' data type from {} to {} for Convolution operation".format(
in_node_1.soft_get('name', in_node_1.id), in_type_1, in_type_0),
extra={'is_warning': True})
convert_const_node_value_type(in_node_1, in_type_0)
Copy link
Contributor

Choose a reason for hiding this comment

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

What cases exactly does this logic cover? input and weight types?

rkazants
rkazants reviewed Dec 22, 2021
View reviewed changes
Copy link
Contributor

@rkazants rkazants left a comment •
edited
Loading

Choose a reason for hiding this comment

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

In general, it looks great. Remained some comments not so relevant now if we get normal DLB results. Let's anticipate DLB and merge if it is fine.

@mvafin mvafin requested a review from a team December 23, 2021 10:26
@rkazants rkazants merged commit 3f35e2a into openvinotoolkit:master Dec 24, 2021
@mvafin mvafin deleted the mo/mixed_precision branch March 14, 2022 12:43
tadamowicz pushed a commit to tadamowicz/openvino that referenced this pull request Aug 30, 2023
@mvafin @dood-apo
Enable new FP16 and support mixed precision by MO (openvinotoolkit#8514)
0ddd7bd 
* Enable new FP16 format and support mixed precision

* Apply review comments

* Fix issue with fp64 in FakeQuantWithMinMaxVars.py

* Enabme decompression converts fusing for CPU plugin

* Apply review feedback

* Fix code style

* Fix issue with np.full and apply review feedback

* Apply review feedback

* Fix HardSigmoid onnx extractor

* Replace np.arrays that were skipped with mo_array

* Fix compress_quantized_weights_test.py

* Fix import issues

* Apply review feedback and fix type of fusing linops in MO

* Apply review feedback

* Fix types for Mean/Scales and MXNET zeros

* Add RandomUniform_8 to ConvertPrecision

* Fix merge issue

* Fix consts names collision in GPU plugin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jane-intel jane-intel jane-intel approved these changes

@nkogteva nkogteva nkogteva approved these changes

@KodiaqQ KodiaqQ KodiaqQ approved these changes

@rkazants rkazants rkazants approved these changes

@AntonDudchenko AntonDudchenko AntonDudchenko approved these changes

@vinograd47 vinograd47 Awaiting requested review from vinograd47

@ArtemySkrebkov ArtemySkrebkov Awaiting requested review from ArtemySkrebkov

@vladimir-paramuzov vladimir-paramuzov Awaiting requested review from vladimir-paramuzov

Assignees

@jane-intel jane-intel

Labels
category: MO Model Optimizer
Projects
None yet
Milestone
2022.1
Development

Successfully merging this pull request may close these issues.
9 participants
@mvafin @Maxim-Doronin @ArtemySkrebkov @rkazants @nkogteva @KodiaqQ @AntonDudchenko @jane-intel @openvino-pushbot