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

Fixed framework name attribute in mapping file. #5046

Merged

Conversation

popovaan
Copy link
Contributor

@popovaan popovaan commented Mar 30, 2021

Root cause analysis:
For some frameworks original operation name was saved incorrectly. Before adding to "fw_tensor_debug_info" it was passed to graph.unique_id() which added id in case if graph has a node with the same name.
Another issue was that in mxnet models tensor names of output nodes were not saved to IR. This happed due to absence of output edges on output nodes where tensor information is stored.
Solution:
Store original framework name in "fw_tensor_debug_info" attribute without passing through graph.unique_id().
Add identity nodes to output nodes in mxnet loader for storing tensor information. These nodes are removed when Result nodes are added to graph.

Ticket: 52131

Code:

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

Validation:

  • Unit tests
  • Framework operation tests - N/A
  • Transformation tests - N/A
  • 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

@popovaan popovaan requested a review from a team March 30, 2021 18:12
@popovaan popovaan marked this pull request as draft March 30, 2021 18:17
@openvino-pushbot openvino-pushbot added the category: MO Model Optimizer label Mar 30, 2021
@popovaan popovaan marked this pull request as ready for review April 5, 2021 06:17
@popovaan popovaan requested review from a team, pavel-esir, mvafin and sadolini and removed request for a team April 5, 2021 06:17
Copy link
Contributor

@mvafin mvafin left a comment

Choose a reason for hiding this comment

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

Generally looks good to me

model-optimizer/mo/front/extractor.py Show resolved Hide resolved
@popovaan popovaan requested a review from mvafin April 6, 2021 12:34
model-optimizer/mo/front/caffe/loader.py Outdated Show resolved Hide resolved
model-optimizer/mo/front/mxnet/extractors/utils.py Outdated Show resolved Hide resolved
model-optimizer/mo/front/onnx/loader.py Outdated Show resolved Hide resolved
model-optimizer/mo/front/mxnet/loader.py Outdated Show resolved Hide resolved
model-optimizer/mo/front/mxnet/loader.py Outdated Show resolved Hide resolved
@lazarevevgeny lazarevevgeny requested a review from iimironov April 8, 2021 09:27
@lazarevevgeny
Copy link
Contributor

@iimironov , please, review the MxNet part.

@lazarevevgeny
Copy link
Contributor

@popovaan, also we need init tests to cover new changes. Consider adding MO tests in the tests-private repo.

@popovaan popovaan requested a review from lazarevevgeny April 8, 2021 14:55
@lazarevevgeny lazarevevgeny merged commit cadff03 into openvinotoolkit:master Apr 8, 2021
mryzhov pushed a commit to mryzhov/openvino that referenced this pull request Apr 23, 2021
* Fixed framework name attribute for onnx, mxnet.

* Fixed framework name attribute for caffe.

* Removed unnecessary attribute setting from add_opoutput()

* Added identity nodes adding to outputs in mxnet loader.

* Removed unnecessary reformat.

* Removed unnecessary reformat.

* Added check for empty name.

* Used nodes indices instead of node names in loader.

* Code refactoring, small bug fixed.
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