RandomUniformFusion transformation. #7187
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openvino-pushbot
added
category: IE Tests
ilyachur
reviewed
Aug 24, 2021
...ngine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp
Outdated
Show resolved
Hide resolved
|
LGTM |
achetver
approved these changes
Aug 24, 2021
rkazants
reviewed
Aug 26, 2021
...ngine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp
Show resolved
Hide resolved
…ons for general min and max value case.
mvafin
reviewed
Sep 7, 2021
...ence-engine/tests/functional/inference_engine/transformations/random_uniform_fusion_test.cpp
Outdated
Show resolved
Hide resolved
GlebKazantaev
suggested changes
Sep 7, 2021
...ngine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp
Outdated
Show resolved
Hide resolved
...ngine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp
Outdated
Show resolved
Hide resolved
...engine/src/transformations/src/transformations/common_optimizations/common_optimizations.cpp
Show resolved
Hide resolved
...ngine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp
Outdated
Show resolved
Hide resolved
...ence-engine/tests/functional/inference_engine/transformations/random_uniform_fusion_test.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Gleb Kazantaev <[email protected]>
rkazants
approved these changes
Sep 13, 2021
GlebKazantaev
suggested changes
Sep 14, 2021
...ngine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp
Outdated
Show resolved
Hide resolved
...ngine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp
Outdated
Show resolved
Hide resolved
...ngine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp
Outdated
Show resolved
Hide resolved
...ngine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp
Outdated
Show resolved
Hide resolved
|
Also, why this transformation depends on LinOpSequenceFusion if it supports both RU->Add and RU->Mul fusions. So the order of Mul and Add operations after RU doesn't matter. |
Because I execute RandomUniformMulFusion first, than RandomUniformAddFusion after it. If we have RU -> Add -> Mul, than only Add will be fused. |
Co-authored-by: Gleb Kazantaev <[email protected]>
Discussed this offline. Yes, this transformation doesn't depend on LinOpSequenceFusion. |
…povaan/openvino into random_uniform_transformation
GlebKazantaev
suggested changes
Sep 14, 2021
...e/src/transformations/include/transformations/common_optimizations/random_uniform_fusion.hpp
Outdated
Show resolved
Hide resolved
...ngine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp
Outdated
Show resolved
Hide resolved
…n_optimizations/random_uniform_fusion.cpp Co-authored-by: Gleb Kazantaev <[email protected]>
…povaan/openvino into random_uniform_transformation
GlebKazantaev
approved these changes
Sep 15, 2021
akuporos
pushed a commit
to akuporos/openvino
that referenced
this pull request
Sep 29, 2021
* Added RandomUniformFusion transformation. * Extended transformations for case with Convert, extended transformations for general min and max value case. * Set to const unchanged variables. * Apply suggestions from code review Co-authored-by: Gleb Kazantaev <[email protected]> * Reformat code, small corrections. * Added const shape checks. * Fixed transformation for case of different const ranks. * Added type checks. * Apply suggestions from code review Co-authored-by: Gleb Kazantaev <[email protected]> * United RandomUniformMulFusion and RandomUniformAddFusion to single transformation. * Added negative tests. * Used get_constant_from_source(). * Moved transformation to common fusions. * Added const refs. * Update inference-engine/src/transformations/src/transformations/common_optimizations/random_uniform_fusion.cpp Co-authored-by: Gleb Kazantaev <[email protected]> * Changed to single class. * Corrected IRs checks in layer tests. * Small corrections. Co-authored-by: Gleb Kazantaev <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details:
This pattern is needed for RandomUniform with floating output type and min != 0 or max != 1. In this case Tensorflow generates following subgraph:
After LinOpSequenceFusion we have the following graph:
Mul and Add in this case can be moved to min and max inputs of RandomUniform.
This transformation is valid only for RandomUniform with floating output type.
In case if Convert goes after RandomUniform Convert should also have floating type.
Tickets: