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

Implement transformation for TensorFlow 2 Map Function (aka tf.map_fn) #6836

Conversation

rkazants
Copy link
Contributor

@rkazants rkazants commented Jul 28, 2021

Root cause analysis: In several TF2 OD models (MobileNet SSD V2, CenterNet) we have a preprocessor block based on tf.map_fn primitive that slices multiple inputs, transform them, and concatenate multiple intermediate outputs at each iteration of While Loop. Currently, the MO tool does not support such block.

Solution: Extend existing transformations for TF2 Keras RNN operations that are particular case of tf.map_fn when it has just one sliced input and one concatenated output. The proposed extension detects sub-graphs (from the main-graph and body-graph) for slicing and concatenation and utilizes axis attribute of Loop operation for such operations.

Ticket: 47280

Code:

  • Comments
  • Code style (PEP8)
  • Transformation generates reshape-able IR
  • Transformation preserves original framework node names

Validation:

  • Unit tests - N/A due to complexity of such unit-tests implementation
  • Framework operation tests
  • Transformation tests - N/A due to complexity of such unit-tests implementation. Now it covers by TF2 Keras RNN layer tests
  • Model Optimizer IR Reader check

Documentation:

  • Supported frameworks operations list - N/A
  • Guide on how to convert the public model - N/A
  • User guide update - N/A

@rkazants rkazants requested review from a team, iimironov, pavel-esir and achetver July 28, 2021 08:56
@openvino-pushbot openvino-pushbot added the category: MO Model Optimizer label Jul 28, 2021
Copy link
Contributor

@achetver achetver left a comment

Choose a reason for hiding this comment

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

Looks good to me

@rkazants rkazants requested a review from lazarevevgeny July 28, 2021 09:45
Copy link
Contributor

@lazarevevgeny lazarevevgeny left a comment

Choose a reason for hiding this comment

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

There are a number of minor comments, but in general looks good. However, I would really want to see a new layer test covering the added functionality:

  1. A test case where a model has one input which is consumed with the TF While operation which produces 2 outputs.
  2. A test case where a model has two inputs which are consumed with the TF While operation which produces 2 outputs.

If one of these test cases is already covered with the existing Keras RNN-like tests that is fine. But the case which was not previously supported should be added to the pre-commit layer tests.

@rkazants
Copy link
Contributor Author

There are a number of minor comments, but in general looks good. However, I would really want to see a new layer test covering the added functionality:

  1. A test case where a model has one input which is consumed with the TF While operation which produces 2 outputs.
  2. A test case where a model has two inputs which are consumed with the TF While operation which produces 2 outputs.

If one of these test cases is already covered with the existing Keras RNN-like tests that is fine. But the case which was not previously supported should be added to the pre-commit layer tests.

Thanks for your point. I will add it.

@rkazants rkazants requested a review from lazarevevgeny July 29, 2021 09:46
@rkazants rkazants merged commit 032cebb into openvinotoolkit:master Jul 30, 2021
rnugmanx pushed a commit to rnugmanx/openvino that referenced this pull request Aug 26, 2021
openvinotoolkit#6836)

* Implement transformation for TensorFlow 2 Map Function primitive

Signed-off-by: Roman Kazantsev <[email protected]>

* Add a description for get_external_node_by_internal_id function

Signed-off-by: Roman Kazantsev <[email protected]>

* Correct a name for get_external_nodes_by_internal_id function

Signed-off-by: Roman Kazantsev <[email protected]>

* Fix a description for get_external_nodes_by_internal_id function

Signed-off-by: Roman Kazantsev <[email protected]>

* Add logging and fix indentation

Signed-off-by: Roman Kazantsev <[email protected]>

* Use skip_nodes_by_condition to by-pass StopGradient nodes for tf.map_fn

Signed-off-by: Roman Kazantsev <[email protected]>
andrei-cv pushed a commit to andrei-cv/openvino that referenced this pull request Aug 30, 2021
openvinotoolkit#6836)

* Implement transformation for TensorFlow 2 Map Function primitive

Signed-off-by: Roman Kazantsev <[email protected]>

* Add a description for get_external_node_by_internal_id function

Signed-off-by: Roman Kazantsev <[email protected]>

* Correct a name for get_external_nodes_by_internal_id function

Signed-off-by: Roman Kazantsev <[email protected]>

* Fix a description for get_external_nodes_by_internal_id function

Signed-off-by: Roman Kazantsev <[email protected]>

* Add logging and fix indentation

Signed-off-by: Roman Kazantsev <[email protected]>

* Use skip_nodes_by_condition to by-pass StopGradient nodes for tf.map_fn

Signed-off-by: Roman Kazantsev <[email protected]>
akuporos pushed a commit to akuporos/openvino that referenced this pull request Sep 29, 2021
openvinotoolkit#6836)

* Implement transformation for TensorFlow 2 Map Function primitive

Signed-off-by: Roman Kazantsev <[email protected]>

* Add a description for get_external_node_by_internal_id function

Signed-off-by: Roman Kazantsev <[email protected]>

* Correct a name for get_external_nodes_by_internal_id function

Signed-off-by: Roman Kazantsev <[email protected]>

* Fix a description for get_external_nodes_by_internal_id function

Signed-off-by: Roman Kazantsev <[email protected]>

* Add logging and fix indentation

Signed-off-by: Roman Kazantsev <[email protected]>

* Use skip_nodes_by_condition to by-pass StopGradient nodes for tf.map_fn

Signed-off-by: Roman Kazantsev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: MO Model Optimizer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants