Reshape able slice #1241

pavel-esir · 2020-07-07T07:40:54Z

Description:

freshened Slice extractor for Caffe, now it's CaffeSlice
added AttributedSlice for ONNX opset < 10 models
added TFSlice internal operation for Slice operation in TF; needed because semantic is different that in ONNX
removed attributes of internal Slice operation: now it's has exact semantic as ONNX Slice opset >= 10
corrected SliceConverter to support reshape-able Slic, no need for multiple nested ifs

~~Cherry-pick form here #1194~~
~~- removed Slice operation in Kaldi~~
~~- parallel component in Kaldi is supported by Split operation~~
previous paragraph is not actual anymore, mentioned PR was merged

#34041

Code:

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

Validation:

Unit tests:
Framework operation tests
Transformation tests
e2e model test with an update of ./tests/e2e_oss/utils/reshape_utils.py: N/A -- no new models added to e2e tests
Model Optimizer IR Reader check

Documentation:

Supported frameworks operations list: N/A
Supported public models list: N/A
New operations specification: N/A
Guide on how to convert the public model: N/A
User guide update: N/A

model-optimizer/extensions/front/caffe/slice_ext.py

model-optimizer/extensions/front/onnx/slice_ext.py

model-optimizer/extensions/front/onnx/slice_ext_test.py

model-optimizer/extensions/front/tf/slice_ext.py

model-optimizer/extensions/front/tf/slice_replacers.py

model-optimizer/mo/ops/slice.py

model-optimizer/mo/utils/unittest/graph.py

model-optimizer/extensions/front/caffe/slice_ext.py

model-optimizer/mo/ops/slice.py

model-optimizer/mo/utils/unittest/graph.py

model-optimizer/mo/utils/shape.py

model-optimizer/mo/ops/slice.py

model-optimizer/mo/ops/slice_test.py

model-optimizer/extensions/front/onnx/AttributedSliceToSlice_test.py

lazarevevgeny · 2020-08-03T19:16:08Z

@jane-intel , please, review the PR.

model-optimizer/mo/ops/slice.py

model-optimizer/mo/utils/shape.py

model-optimizer/mo/utils/unittest/graph.py

lazarevevgeny · 2020-08-05T08:28:18Z

@jane-intel, please, take a look.

…corrected SliceConverter and added unittests for all cases

…ence; moved mxlice inside of slice.py; renamed slice_replacers

…s fro slice_replacer onnx phase

…pe calculation, and some other minor corrections

lazarevevgeny

LGTM. @jane-intel, please, have a fresh look at this PR.

model-optimizer/extensions/front/onnx/AttributedSliceToSlice.py

jane-intel · 2020-08-06T07:42:37Z

model-optimizer/extensions/front/tf/TFSliceToSlice.py

+
+    def replace_sub_graph(self, graph: Graph, match: dict):
+        """
+        This transformation converts TFSlice to internal Slice operation.


Transformation itself is not commented -- what is happening

Moved comments to class level. Please review.

jane-intel · 2020-08-06T07:46:50Z

model-optimizer/mo/ops/slice.py

+
+"""
+Slicing operations have different semantic or different parameters/inputs in different frameworks. To distinguish them 
+and not confuse with OpenVINO Slice several Model Optimizer internal operations are introduced. OpenVINO Slice operation 


there is no OpenVINO Slice operation. We should not confuse people

Agree. Clarified this. Please review.

jane-intel · 2020-08-06T07:51:07Z

model-optimizer/mo/ops/slice.py



 class Slice(Op):
+    """
+    Semantic of OpenVINO Slice operation is identical ONNX Slice (opset >= 10).


There is no OpenVINO Slice operation. It is not in public opset

Updated comments. Please review.

jane-intel · 2020-08-07T06:46:47Z

model-optimizer/mo/ops/slice.py


-    def __init__(self, graph: Graph, attrs: dict):
+    def __init__(self, graph: Graph, attrs: dict = None):


Here you set arrts to None, but everywhere else -- we don't do this. What was original reason to do so?

I added this default value to avoid specifying when I call constrictor, e.g. here TFSliceToSlice.py:41
slice_node = Slice(graph).create_node()

At the moment it looks like it's the only place where Slice constructor is called explicitly. I guess in the draft versions of PR i was using it more often and it remained here for historical reason. I advocate to leave this default argument here.

model-optimizer/mo/ops/slice.py

* Added Caffe Slice_ext * Added TFSlice, AttributedSlice (both with extractors and replacers), corrected SliceConverter and added unittests for all cases * added comments to each type of Slice operation; optimized shape inference; moved mxlice inside of slice.py; renamed slice_replacers * removed type annotation for get_shape_after_slice routine * replaced zeros_like with zeros * Corrected preserving node names, renamed attributes names, added tests fro slice_replacer onnx phase * Renamed slice_replacers.py * added more unittest cases * added type annotations, moved to more relevant place routines for shape calculation, and some other minor corrections * corrected a typo `normalize_slice_indices` comment * corrected shape calculation for Nonconstant inputs * corrected a few typos * corrected type declarations * corrected shape inference with rounding * refactored unit-tests for front transforms of Slice * added error raising for negative and zero shapes * removed magic_num * corrected AttributedSlice, clarified comments * fixed unit-test for AttributedSliceToSlice * typo in type hints corrected * removed supported_attrs * returned back default None for attrs of Slice

pavel-esir requested review from lazarevevgeny, vgavrilo and jane-intel July 7, 2020 07:40

vgavrilo approved these changes Jul 7, 2020

View reviewed changes

pavel-esir force-pushed the reshape-able-slice branch from 61937ba to e43d33b Compare July 7, 2020 16:07

pavel-esir marked this pull request as ready for review July 7, 2020 16:09

pavel-esir requested review from a team and vgavrilo July 7, 2020 16:09

vgavrilo approved these changes Jul 8, 2020

View reviewed changes

ilya-lavrenov added the category: MO Model Optimizer label Jul 8, 2020

ilya-lavrenov added this to the 2021.1 milestone Jul 8, 2020

lazarevevgeny suggested changes Jul 9, 2020

View reviewed changes

pavel-esir force-pushed the reshape-able-slice branch 6 times, most recently from 26b3843 to a76ebcf Compare July 20, 2020 11:11

pavel-esir force-pushed the reshape-able-slice branch from a76ebcf to 322142e Compare July 23, 2020 11:36

pavel-esir mentioned this pull request Jul 23, 2020

support parallel nested nnet for Kaldi #1194

Merged

14 tasks

pavel-esir force-pushed the reshape-able-slice branch from 322142e to 4dfa2f3 Compare July 23, 2020 13:53

lazarevevgeny suggested changes Jul 24, 2020

View reviewed changes

pavel-esir force-pushed the reshape-able-slice branch 2 times, most recently from 55eadc5 to 0fd6d56 Compare August 2, 2020 17:14

lazarevevgeny suggested changes Aug 3, 2020

View reviewed changes

model-optimizer/mo/ops/slice.py Outdated Show resolved Hide resolved

model-optimizer/mo/utils/shape.py Outdated Show resolved Hide resolved

model-optimizer/mo/utils/unittest/graph.py Outdated Show resolved Hide resolved

pavel-esir added 3 commits August 5, 2020 18:18

Added Caffe Slice_ext

7bbd861

Added TFSlice, AttributedSlice (both with extractors and replacers), …

be9b281

…corrected SliceConverter and added unittests for all cases

added comments to each type of Slice operation; optimized shape infer…

8623741

…ence; moved mxlice inside of slice.py; renamed slice_replacers

pavel-esir added 11 commits August 5, 2020 18:18

replaced zeros_like with zeros

052833f

Corrected preserving node names, renamed attributes names, added test…

287216c

…s fro slice_replacer onnx phase

Renamed slice_replacers.py

683d2c3

added more unittest cases

876801b

added type annotations, moved to more relevant place routines for sha…

d9606a5

…pe calculation, and some other minor corrections

corrected a typo normalize_slice_indices comment

e2d91ef

corrected shape calculation for Nonconstant inputs

026750d

corrected a few typos

89cb9f3

corrected type declarations

ce79359

corrected shape inference with rounding

724b44e

refactored unit-tests for front transforms of Slice

a28d5bc

pavel-esir force-pushed the reshape-able-slice branch from a8730d3 to a28d5bc Compare August 5, 2020 15:50

added error raising for negative and zero shapes

4335670

lazarevevgeny approved these changes Aug 5, 2020

View reviewed changes

removed magic_num

d9c9847

jane-intel reviewed Aug 6, 2020

View reviewed changes

pavel-esir added 3 commits August 6, 2020 16:08

corrected AttributedSlice, clarified comments

57df332

fixed unit-test for AttributedSliceToSlice

ab6b005

typo in type hints corrected

9c23fab

jane-intel reviewed Aug 7, 2020

View reviewed changes

pavel-esir added 2 commits August 7, 2020 12:27

removed supported_attrs

0278a7c

returned back default None for attrs of Slice

831be1d

pavel-esir requested a review from jane-intel August 7, 2020 12:59

jane-intel approved these changes Aug 10, 2020

View reviewed changes

lazarevevgeny merged commit 75d2d88 into openvinotoolkit:master Aug 10, 2020

pavel-esir deleted the reshape-able-slice branch September 18, 2020 08:57

pavel-esir added bug Something isn't working and removed bug Something isn't working labels May 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reshape able slice #1241

Reshape able slice #1241

pavel-esir commented Jul 7, 2020 •

edited

Loading

lazarevevgeny commented Aug 3, 2020

lazarevevgeny commented Aug 5, 2020

lazarevevgeny left a comment

jane-intel Aug 6, 2020

pavel-esir Aug 6, 2020

jane-intel Aug 6, 2020

pavel-esir Aug 6, 2020

jane-intel Aug 6, 2020

pavel-esir Aug 6, 2020

jane-intel Aug 7, 2020

pavel-esir Aug 7, 2020


		def __init__(self, graph: Graph, attrs: dict):
		def __init__(self, graph: Graph, attrs: dict = None):

Reshape able slice #1241

Reshape able slice #1241

Conversation

pavel-esir commented Jul 7, 2020 • edited Loading

lazarevevgeny commented Aug 3, 2020

lazarevevgeny commented Aug 5, 2020

lazarevevgeny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pavel-esir commented Jul 7, 2020 •

edited

Loading