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.

Sign up for GitHub

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

Jump to bottom

[MO] Add support to moc_frontend of ":" as delimiter for --input and --output #6543

Merged
merged 12 commits into from
Jul 29, 2021
Merged

[MO] Add support to moc_frontend of ":" as delimiter for --input and --output #6543

merged 12 commits into from
Jul 29, 2021

Conversation

nosovmik
Copy link
Contributor

@nosovmik nosovmik commented Jul 6, 2021

Details:

[MO] Add support to moc_frontend of ":" as delimiter for --input

For input like "0:1" the following possibilities are considered according to discussion:

  1. Tensor with name "0:1"
  2. Operation with name "0:1"
  3. Operation "0" with output port = 1
  4. Operation "1" with input port = 0
    Collisions are detected using 'Place.is_equal_data' to check if place points to same data or not

Error codes are inherited from original 'extractor' implementation

Additional changes:
Changed default logic for 'Place::get_in(out)put_port' to return nullptr
Changed default logic for 'InputModel::get_place_by_tensor(operation)_name' to return nullptr

Ticket: 59409

Code:

  • [N/A ] Comments (code is self-explained)
  • Code style (PEP8)
  • [N/A] Transformation generates reshape-able IR
  • [N/A] Transformation preserves original framework node names

Validation:

  • Unit tests
  • [N/A] Framework operation tests
  • [N/A] Transformation tests
  • [N/A] Model Optimizer IR Reader check

Documentation:

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

Validation

A number of unit tests were implemented for 'moc_frontend/extractor'. Tests (15 in total) cover all possible scenarios when legacy style with ":" is used

@nosovmik nosovmik changed the title [MO] Add support to moc_frontend of ":" as delimiter for --input [MO] Add support to moc_frontend of ":" as delimiter for --input and --output Jul 6, 2021
@openvino-pushbot openvino-pushbot added category: MO Model Optimizer category: Core OpenVINO Core (aka ngraph) labels Jul 6, 2021
nosovmik added 4 commits July 6, 2021 20:28
@nosovmik
[MO] Add support to moc_frontend of ":" as delimiter for --input
3c0101c 
Additions:
Changed default logic for 'Place::get_in(out)put_port' to return nullptr
Changed default logic for 'InputModel::get_place_by_tensor(operation)_name' to return nullptr
@nosovmik
Corrected comments in code
3f1393f
@nosovmik
Missing empty line
8126c28
@nosovmik
Clang format fixes
39fc549
@nosovmik nosovmik requested review from ilyachur, jane-intel and a team July 7, 2021 07:34
@nosovmik nosovmik requested a review from rkazants July 12, 2021 08:32
nosovmik added 6 commits July 19, 2021 12:40
@nosovmik
Merge branch 'openvinotoolkit:master' into mo/input_port
e78186f
@nosovmik
Fix review comments
5f6d234
@nosovmik
Updated test to verify review comments fixes
3f58ff7
@nosovmik
Merge remote-tracking branch 'upstream/master' into mo/input_port
dea09dd
@nosovmik
Merge remote-tracking branch 'upstream/master' into mo/input_port
c23a4a0 
# Conflicts:
#	ngraph/frontend/frontend_manager/include/frontend_manager/input_model.hpp
#	ngraph/frontend/frontend_manager/include/frontend_manager/place.hpp
#	ngraph/frontend/frontend_manager/src/frontend_manager.cpp
#	ngraph/test/frontend/frontend_manager.cpp
@nosovmik
Update unit tests after rebase
9488d34
@nosovmik nosovmik requested a review from rkazants July 26, 2021 14:55
@nosovmik
Copy link
Contributor Author

@lazarevevgeny @rkazants @mvafin can you please review this as MO developers?

lazarevevgeny
lazarevevgeny suggested changes Jul 28, 2021
View reviewed changes
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.

The logic in the MO code looks good. However, I have added a number of nitpick comments and raised a concern about hardcoding of test values in the unit test infrastructure code.

model-optimizer/unit_tests/mock_mo_frontend/mock_mo_ngraph_frontend/mock_mo_frontend.hpp Outdated Show resolved Hide resolved
model-optimizer/mo/moc_frontend/extractor.py Outdated Show resolved Hide resolved
model-optimizer/mo/moc_frontend/extractor.py Outdated Show resolved Hide resolved
model-optimizer/mo/moc_frontend/extractor.py Outdated Show resolved Hide resolved
model-optimizer/mo/moc_frontend/extractor.py Outdated Show resolved Hide resolved
@nosovmik nosovmik requested a review from lazarevevgeny July 28, 2021 15:57
@lazarevevgeny lazarevevgeny merged commit 868fad3 into openvinotoolkit:master Jul 29, 2021
akuporos pushed a commit to akuporos/openvino that referenced this pull request Jul 29, 2021
@nosovmik @akuporos
[MO] Add support to moc_frontend of ":" as delimiter for --input and …
2880d82 
…--output (openvinotoolkit#6543)

* [MO] Add support to moc_frontend of ":" as delimiter for --input

Additions:
Changed default logic for 'Place::get_in(out)put_port' to return nullptr
Changed default logic for 'InputModel::get_place_by_tensor(operation)_name' to return nullptr

* Corrected comments in code

* Missing empty line

* Clang format fixes

* Fix review comments

* Updated test to verify review comments fixes

* Update unit tests after rebase

* Apply review comments
rnugmanx pushed a commit to rnugmanx/openvino that referenced this pull request Aug 26, 2021
@nosovmik
[MO] Add support to moc_frontend of ":" as delimiter for --input and …
bbee28d 
…--output (openvinotoolkit#6543)

* [MO] Add support to moc_frontend of ":" as delimiter for --input

Additions:
Changed default logic for 'Place::get_in(out)put_port' to return nullptr
Changed default logic for 'InputModel::get_place_by_tensor(operation)_name' to return nullptr

* Corrected comments in code

* Missing empty line

* Clang format fixes

* Fix review comments

* Updated test to verify review comments fixes

* Update unit tests after rebase

* Apply review comments
andrei-cv pushed a commit to andrei-cv/openvino that referenced this pull request Aug 30, 2021
@nosovmik @andrei-cv
[MO] Add support to moc_frontend of ":" as delimiter for --input and …
76bf325 
…--output (openvinotoolkit#6543)

* [MO] Add support to moc_frontend of ":" as delimiter for --input

Additions:
Changed default logic for 'Place::get_in(out)put_port' to return nullptr
Changed default logic for 'InputModel::get_place_by_tensor(operation)_name' to return nullptr

* Corrected comments in code

* Missing empty line

* Clang format fixes

* Fix review comments

* Updated test to verify review comments fixes

* Update unit tests after rebase

* Apply review comments
akuporos pushed a commit to akuporos/openvino that referenced this pull request Sep 29, 2021
@nosovmik @akuporos
[MO] Add support to moc_frontend of ":" as delimiter for --input and …
81fdb8a 
…--output (openvinotoolkit#6543)

* [MO] Add support to moc_frontend of ":" as delimiter for --input

Additions:
Changed default logic for 'Place::get_in(out)put_port' to return nullptr
Changed default logic for 'InputModel::get_place_by_tensor(operation)_name' to return nullptr

* Corrected comments in code

* Missing empty line

* Clang format fixes

* Fix review comments

* Updated test to verify review comments fixes

* Update unit tests after rebase

* Apply review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rkazants rkazants rkazants approved these changes

@lazarevevgeny lazarevevgeny lazarevevgeny approved these changes

@mvafin mvafin Awaiting requested review from mvafin

@GlebKazantaev GlebKazantaev Awaiting requested review from GlebKazantaev

@slyalin slyalin Awaiting requested review from slyalin

@ilyachur ilyachur Awaiting requested review from ilyachur

@jane-intel jane-intel Awaiting requested review from jane-intel

Assignees
No one assigned
Labels
category: Core OpenVINO Core (aka ngraph) category: MO Model Optimizer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nosovmik @lazarevevgeny @rkazants @openvino-pushbot