Skip to content

Fixed transformation to pull constants into Loop body#4591

Merged
lazarevevgeny merged 7 commits intoopenvinotoolkit:masterfrom
lazarevevgeny:fix_pulling_constants_into_loop
Mar 4, 2021
Merged

Fixed transformation to pull constants into Loop body#4591
lazarevevgeny merged 7 commits intoopenvinotoolkit:masterfrom
lazarevevgeny:fix_pulling_constants_into_loop

Conversation

@lazarevevgeny
Copy link
Copy Markdown
Contributor

@lazarevevgeny lazarevevgeny commented Mar 3, 2021

Description: Added additional check to the function which pulls Const input into the Loop body. This check make sure that if there is a back edge to the body Parameter which gets a Const as input (from the main graph) then the value of this tensor is not changed during the iteration.

Ticket: 50269

Code:

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

Validation:

  • Unit tests
  • Framework operation tests: No new layers enabled
  • Transformation tests: manually checked TF 2.X GRU layer tests
  • Model Optimizer IR Reader check: Done with the failed model

Documentation:

  • Supported frameworks operations list: N/A no new operation enabled
  • Guide on how to convert the public model: N/A no new model enabled
  • User guide update: N/A no user visible changes

@lazarevevgeny lazarevevgeny marked this pull request as ready for review March 3, 2021 18:06
@lazarevevgeny lazarevevgeny requested review from a team, pavel-esir, rkazants and vgavrilo and removed request for a team March 3, 2021 18:06
@lazarevevgeny lazarevevgeny added this to the 2021.3 milestone Mar 3, 2021
@lazarevevgeny lazarevevgeny added bug Something isn't working category: MO Model Optimizer labels Mar 3, 2021
@lazarevevgeny lazarevevgeny modified the milestones: 2021.3, 2021.4 Mar 4, 2021
Copy link
Copy Markdown
Contributor

@pavel-esir pavel-esir left a comment

Choose a reason for hiding this comment

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

Left a comment about a typo. In general looks good to me

Comment on lines +331 to +332
if not any([attr['to_layer'] == body_parameter.soft_get('internal_layer_id') for attr in loop_node.back_edges]):
return True
Copy link
Copy Markdown
Collaborator

@rkazants rkazants Mar 4, 2021

Choose a reason for hiding this comment

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

nit: we know that the number of back edges is equal either to 0 or 1:

needed_back_edges = [attr['to_layer'] == body_parameter.soft_get('internal_layer_id') for attr in loop_node.back_edges]
if len(needed_back_edges) == 0:
    return True
assert len(needed_back_edges) == 1

back_edge_attrs = needed_back_edges[0]
result_internal_id = back_edge_attrs['from_layer']
result_nodes = loop_node.body.get_op_nodes(internal_layer_id=result_internal_id)
assert len(result_nodes) == 1, 'There should be exactly one node with id {}, but there are {}' \
                               ''.format(result_internal_id, len(result_nodes))
result_node = result_nodes[0]
parameters = common_bfs(result_node, ['Identity'], ['Parameter'], is_backward=True, attr_to_check='op',
                        follow_multi_consumer_data_nodes=True)
if any([node.soft_get('internal_layer_id') == body_parameter.internal_layer_id for node in parameters]):
    return True

And you do not need for loop and if check then

@rkazants
Copy link
Copy Markdown
Collaborator

rkazants commented Mar 4, 2021

The TF2 RNN layer tests pass as expected and no new failure observed.

@lazarevevgeny lazarevevgeny merged commit 7c5c708 into openvinotoolkit:master Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working category: MO Model Optimizer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants