Skip to content

Corrected SSD text graph generation for Identity nodes#19417

Merged
alalek merged 2 commits intoopencv:3.4from
LupusSanctus:am/text_graph_identity
Feb 17, 2021
Merged

Corrected SSD text graph generation for Identity nodes#19417
alalek merged 2 commits intoopencv:3.4from
LupusSanctus:am/text_graph_identity

Conversation

@LupusSanctus
Copy link
Copy Markdown

@LupusSanctus LupusSanctus commented Jan 28, 2021

Resolves: #17484

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@LupusSanctus LupusSanctus force-pushed the am/text_graph_identity branch from 2158e81 to fd921a0 Compare January 28, 2021 18:05
@LupusSanctus LupusSanctus changed the base branch from master to 3.4 January 28, 2021 18:06
@asmorkalov asmorkalov requested a review from dkurt January 28, 2021 19:09
if node.input[0] in identities:
identities[node.name] = identities[node.input[0]]
else:
identities[node.name] = node.input[0]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it make sense to make it more generic:

inp = node.input[0]
while inp in identities:
  inp = identities[inp]
identities[node.name] = inp

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@dkurt, thank you for the review!
However, is there a possibility of random graph traversal or graph traversal from child nodes to parent or some particular Identity sequence case? If for node in graph_def.node: determines descending traversal from a parent node, then we don't need an extra while inp in identities: loop because we sequentially handle nodes and if there are two or more nodes of Identity type (ex.: a -> identity_0 [inp: a] -> identity_1 [inp: identity_0] -> ... -> identity_n [inp: identity_n-1] -> b [inp: identity_n]), we will check the presence of current identity node input in identities = {} and if it's there, we will add the current identity node to the identities = {}, overwriting its input value with the input value of initial identity.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

At this moment nodes order is sorted by execution (by writeTextGraph method).

@LupusSanctus LupusSanctus force-pushed the am/text_graph_identity branch 2 times, most recently from 2158e81 to 408e8e4 Compare February 8, 2021 11:16
Copy link
Copy Markdown
Member

@dkurt dkurt left a comment

Choose a reason for hiding this comment

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

👍

@alalek alalek merged commit eb90186 into opencv:3.4 Feb 17, 2021
@alalek alalek mentioned this pull request Feb 21, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tf_text_graph_ssd.py produce invalid text-graph representation

3 participants