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

nGraph 'shell' implementation for GatherElements-6 and MO 'shell' implementation #3467

Merged
merged 21 commits into from
Dec 21, 2020
Merged

nGraph 'shell' implementation for GatherElements-6 and MO 'shell' implementation #3467

merged 21 commits into from
Dec 21, 2020

Conversation

pavel-esir
Copy link
Contributor

Description: Added 'shell' implementation of GatherElement-6 operation to nGraph and MO. By MO 'shell' implementation I mean that value_inference in MO is not implemented yet.

tickets: #42630, #42628

The following tasks are applicable only for MO part of PR. If you are not reviewing MO part you can ignore them.

Code:

  • Comments
  • Code style (PEP8)
  • Transformation generates reshape-able IR: N/A -- no new transformations were added
  • Transformation preserves original framework node names: N/A -- no new transformation were added

Validation:

  • Unit tests:
  • Framework operation tests: layer test are not implemented yet but checks have done manually with different inputs and attributes and results matched to FW
  • Transformation tests: N/A -- no new transformations were added
  • e2e model test with an update of ./tests/e2e_oss/utils/reshape_utils.py: N/A
  • Model Optimizer IR Reader check

Documentation:

  • Supported frameworks operations list
  • Supported public models list: N/A
  • New operations specification: Spec for GatherElements-6 #3304
  • Guide on how to convert the public model: N/A
  • User guide update: N/A

@pavel-esir pavel-esir added category: MO Model Optimizer category: Core OpenVINO Core (aka ngraph) labels Dec 3, 2020
@pavel-esir pavel-esir added this to the 2021.3 milestone Dec 3, 2020
@openvino-pushbot openvino-pushbot added the category: docs OpenVINO documentation label Dec 3, 2020
@pavel-esir pavel-esir force-pushed the add_gather_elements_6_to_mo_ngraph branch from 0d61833 to 5f6bc73 Compare December 4, 2020 06:13
lazarevevgeny
lazarevevgeny suggested changes Dec 4, 2020
View reviewed changes
model-optimizer/extensions/front/onnx/gatherelements_ext.py Outdated Show resolved Hide resolved
model-optimizer/extensions/ops/gatherelements.py Outdated Show resolved Hide resolved
ngraph/core/include/ngraph/op/gather_elements.hpp Outdated Show resolved Hide resolved
ngraph/core/include/ngraph/op/gather_elements.hpp Outdated Show resolved Hide resolved
ngraph/core/include/ngraph/op/gather_elements.hpp Outdated Show resolved Hide resolved
model-optimizer/extensions/ops/gatherelements.py Outdated Show resolved Hide resolved
rkazants
rkazants requested changes Dec 8, 2020
View reviewed changes
model-optimizer/extensions/ops/gatherelements.py Show resolved Hide resolved
model-optimizer/extensions/ops/gatherelements.py Outdated Show resolved Hide resolved
model-optimizer/extensions/ops/gatherelements.py Outdated Show resolved Hide resolved
ngraph/core/src/op/gather_elements.cpp Outdated Show resolved Hide resolved
ngraph/core/src/op/gather_elements.cpp Outdated Show resolved Hide resolved
ngraph/core/src/op/gather_elements.cpp Outdated Show resolved Hide resolved
@pavel-esir pavel-esir force-pushed the add_gather_elements_6_to_mo_ngraph branch from e692101 to cec0c80 Compare December 10, 2020 08:49
@pavel-esir
Copy link
Contributor Author

pavel-esir commented Dec 10, 2020 •
edited
Loading

Implemented validate_and_infer_types with intervals for dynamic shapes. Solution is general and works for static shapes as well. @lazarevevgeny, @rkazants, @jane-intel please review.

@pavel-esir pavel-esir requested a review from rkazants December 10, 2020 09:12
lazarevevgeny
lazarevevgeny suggested changes Dec 11, 2020
View reviewed changes
model-optimizer/extensions/ops/gatherelements.py Outdated Show resolved Hide resolved
model-optimizer/extensions/ops/gatherelements.py Outdated Show resolved Hide resolved
model-optimizer/extensions/ops/gatherelements.py Outdated Show resolved Hide resolved
model-optimizer/extensions/ops/gatherelements.py Show resolved Hide resolved
ngraph/core/src/op/gather_elements.cpp Outdated Show resolved Hide resolved
ngraph/core/src/op/gather_elements.cpp Outdated Show resolved Hide resolved
@pavel-esir pavel-esir force-pushed the add_gather_elements_6_to_mo_ngraph branch from cec0c80 to 6cb8e50 Compare December 15, 2020 10:01
@pavel-esir pavel-esir force-pushed the add_gather_elements_6_to_mo_ngraph branch 2 times, most recently from ca8a98c to 2bbbca9 Compare December 15, 2020 19:37
pavel-esir and others added 21 commits December 17, 2020 22:39
@pavel-esir
Initial support of GatherElements in MO and nGraph
36cb68c
@pavel-esir
apply_style
17e1651
@pavel-esir
added lost extractor for GatherElements
3bd745d
@pavel-esir
Corrected GatherElements::validate_and_infer_types
8293893
@pavel-esir
updated package_BOM.txt
a1b4d5b
@pavel-esir
Type_t added
788d021
@pavel-esir
started to implement ngraph shape_type_infer unit-tests
e419cd1
@pavel-esir
finally implemented all ngraph shape_inference unit-tests
5e9197d
@pavel-esir
updated Supported_Frameworks_Layers.md
9a67d4d
@pavel-esir
added correct handling of dynamic shapes in nGraph, added unit-tests …
03f1b18 
…for dynamic cases, fixed dump typos in MO, replaced axis type from int -> int64_t
@pavel-esir
implemented shape infer for dynamic shapes with intervals
67cf2bc
@pavel-esir
finalized MO implementation
54513a9
@pavel-esir
applied comment from review
1049dbe
@pavel-esir
style-apply
b563c66
@pavel-esir
spec correction
7d90b79
@pavel-esir
removed conflict
f62b99a
@pavel-esir
fixed typos
1b00249
@pavel-esir
removed obsolete comments form type_prop
67f82c2
@pavel-esir
significant corrections in validate_and_infer_types
35d5302
@pavel-esir
style-apply
59d5c23
@pavel-esir
data_rank check for axis
8b798f6
@pavel-esir pavel-esir force-pushed the add_gather_elements_6_to_mo_ngraph branch from 93e82ad to 8b798f6 Compare December 17, 2020 19:39
@jane-intel jane-intel merged commit d2a2368 into openvinotoolkit:master Dec 21, 2020
@pavel-esir pavel-esir restored the add_gather_elements_6_to_mo_ngraph branch December 22, 2020 13:27
mryzhov pushed a commit to mryzhov/openvino that referenced this pull request Jan 14, 2021
@pavel-esir @mryzhov
nGraph 'shell' implementation for GatherElements-6 and MO 'shell' imp…
86bf405 
…lementation (openvinotoolkit#3467)

* Initial support of GatherElements in MO and nGraph

* apply_style

* added lost extractor for GatherElements

* Corrected GatherElements::validate_and_infer_types

* updated package_BOM.txt

* Type_t added

* started to implement ngraph shape_type_infer unit-tests

* finally implemented all ngraph shape_inference unit-tests

* updated Supported_Frameworks_Layers.md

* added correct handling of dynamic shapes in nGraph, added unit-tests for dynamic cases, fixed dump typos in MO, replaced axis type from int -> int64_t

* implemented shape infer for dynamic shapes with intervals

* finalized MO implementation

* applied comment from review

* style-apply

* spec correction

* removed conflict

* fixed typos

* removed obsolete comments form type_prop

* significant corrections in validate_and_infer_types

* style-apply

* data_rank check for axis
@pavel-esir pavel-esir deleted the add_gather_elements_6_to_mo_ngraph branch February 5, 2021 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jane-intel jane-intel jane-intel approved these changes

@rkazants rkazants rkazants approved these changes

@lazarevevgeny lazarevevgeny lazarevevgeny approved these changes

@ilyachur ilyachur ilyachur approved these changes

@GlebKazantaev GlebKazantaev Awaiting requested review from GlebKazantaev

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

Successfully merging this pull request may close these issues.
6 participants
@pavel-esir @ilyachur @lazarevevgeny @rkazants @jane-intel @openvino-pushbot