[MO] StridedSlice improvements by pavel-esir · Pull Request #4139 · openvinotoolkit/openvino

pavel-esir · 2021-02-02T10:54:46Z

Description: Fixed some issues with StridedSlice permutation and processing ellipsis masks. Now infer function do not change attributes and inputs of StridedSlice, moved normalization into a separate transformation, also reused general existing permute mechanism. Now complex StridedSlices are correctly inferred in ShapeOf subgraphs.

tickets 40372, 43072, 39311

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
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

…cherry-picked maxpoolV4 and tf_broadcast_ext)

…w rewritten SS infer

… code clearing

model-optimizer/mo/utils/ir_reader/extenders/strided_slice_extender.py

model-optimizer/extensions/back/CropToStridedSlice.py

model-optimizer/extensions/middle/ApplyPermutations.py

model-optimizer/mo/front/mxnet/extractors/slice_axis.py

model-optimizer/mo/graph/perm_inputs.py

model-optimizer/mo/front/common/partial_infer/utils.py

model-optimizer/mo/ops/strided_slice.py

model-optimizer/extensions/middle/StridedSliceNormalizer.py

model-optimizer/mo/ops/strided_slice.py

model-optimizer/extensions/middle/StridedSliceNormalizer.py

lazarevevgeny

In general looks good, but still have some comments.

Also, please, run unit test with code coverage to see how new functions are covered with unit tests: coverage run -m unittest discover -p "*_test.py"

model-optimizer/mo/ops/strided_slice.py

model-optimizer/mo/utils/ir_reader/extenders/strided_slice_extender.py

model-optimizer/extensions/middle/StridedSliceNormalizer.py

pavel-esir · 2021-02-15T10:49:00Z

@lazarevevgeny thanks for your comments! Resolved them all.
coverage run -m unittest discover -p "*_test.py" - successfully done

There are 3 sets of automatically unit-tests that cover almost all cases.
ShapeInfer tests with shrink_axis, newaxis and ellipsis placed on almost every possible positions of 2, 3, 4 rank slices with different input_ranks.

openvino/model-optimizer/mo/ops/strided_slice_test.py

Line 511 in 73046d7

    
           # automatically generated the whole range of 2d slices over 2d, 3d and 4d input tensors

The same ShapeInfer tests after Normalizer

openvino/model-optimizer/extensions/middle/StridedSliceNormalizer_test.py

Line 395 in 73046d7

    
           # automatically generated the whole range of 2d slices over 2d, 3d and 4d input tensors

Permute tests

openvino/model-optimizer/extensions/middle/StridedSliceNormalizer_test.py

Line 1660 in 73046d7

# automatically generated permutation tests

dl-benchmark finished without regressions on a previous run. @jane-intel, @sadolini, @rkazants please review

e-nugmanova

First part 14/20 files

model-optimizer/extensions/back/StridedSliceMasksNormalizer.py

model-optimizer/extensions/middle/StridedSliceNormalizer.py

model-optimizer/mo/front/common/partial_infer/utils.py

…ected comments and other minor corrections

model-optimizer/extensions/middle/StridedSliceNormalizer.py

lazarevevgeny

Overall looks good. But left several minor comments to be fixed.

model-optimizer/extensions/middle/StridedSliceNormalizer.py

docs/MO_DG/prepare_model/Supported_Frameworks_Layers.md

likholat · 2021-02-17T15:33:59Z

According to the documentation, the stride input is optional: https://docs.openvinotoolkit.org/latest/openvino_docs_ops_movement_StridedSlice_1.html
But after these changes, the absence of the stride input causes the error:

[ ERROR ]  Traceback (most recent call last):
  File "/home/anna/projects/openvino/model-optimizer/mo/utils/class_registration.py", line 290, in apply_transform
    for_graph_and_each_sub_graph_recursively(graph, replacer.find_and_replace_pattern)
  File "/home/anna/projects/openvino/model-optimizer/mo/middle/pattern_match.py", line 59, in for_graph_and_each_sub_graph_recursively
    func(graph)
  File "/home/anna/projects/openvino/model-optimizer/extensions/middle/StridedSliceNormalizer.py", line 107, in find_and_replace_pattern
    StridedSliceNormalizer.normalize_strided_slice(graph, node)
  File "/home/anna/projects/openvino/model-optimizer/extensions/middle/StridedSliceNormalizer.py", line 129, in normalize_strided_slice
    StridedSliceNormalizer.normalize_slices_attr(node)
  File "/home/anna/projects/openvino/model-optimizer/extensions/middle/StridedSliceNormalizer.py", line 249, in normalize_slices_attr
    res_slices[-1] = slice(*res_slices[-1].indices(data_shape[in_idx]))  # convert negative begins/ends
TypeError: slice indices must be integers or None or have an __index__ method

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/anna/projects/openvino/model-optimizer/mo/main.py", line 305, in main
    ret_code = driver(argv)
  File "/home/anna/projects/openvino/model-optimizer/mo/main.py", line 269, in driver
    ret_res = emit_ir(prepare_ir(argv), argv)
  File "/home/anna/projects/openvino_contrib/modules/mo_pytorch/mo_pytorch.py", line 106, in _prepare_ir
    graph = unified_pipeline(argv)
  File "/home/anna/projects/openvino/model-optimizer/mo/pipeline/unified.py", line 29, in unified_pipeline
    class_registration.ClassType.BACK_REPLACER
  File "/home/anna/projects/openvino/model-optimizer/mo/utils/class_registration.py", line 340, in apply_replacements
    apply_replacements_list(graph, replacers_order)
  File "/home/anna/projects/openvino/model-optimizer/mo/utils/class_registration.py", line 330, in apply_replacements_list
    num_transforms=len(replacers_order))
  File "/home/anna/projects/openvino/model-optimizer/mo/utils/logger.py", line 124, in wrapper
    function(*args, **kwargs)
  File "/home/anna/projects/openvino/model-optimizer/mo/utils/class_registration.py", line 318, in apply_transform
    )) from err
Exception: Exception occurred during running replacer "REPLACEMENT_ID (<class 'extensions.middle.StridedSliceNormalizer.StridedSliceNormalizer'>)": slice indices must be integers or None or have an __index__ method

Is it a bug or the stride input is now required?

pavel-esir · 2021-02-17T19:00:28Z

Is it a bug or the stride input is now required?

Specification haven't been changed. strides are still optional. Looks like a bug. I will take a look

pavel-esir force-pushed the strided_slice_fix branch from 8f9b122 to a19dda7 Compare February 2, 2021 10:55

pavel-esir changed the title ~~StridedSlice~~ [MO] StridedSlice improvements Feb 2, 2021

pavel-esir added 10 commits February 3, 2021 11:49

fix ss

2bce1da

successfully converted

eabf5f3

successfully run moved infer and normalizer unit-tests

166976e

successfully rewritten StridedSlice infer unittests

04df120

int64 array

1486d87

Successfully converter crash-when-loading, xj_feauture and toy nets (…

dde8937

…cherry-picked maxpoolV4 and tf_broadcast_ext)

successfully moved PermuteAttrs to general mechanism

ab3b7b2

successfully converted xj_feauture and crash when loading with the ne…

e9aa2d7

…w rewritten SS infer

fixed get_shape_from_slice and moved to common utils

b7b5dd0

fixed extending masks and some other

e79d72f

pavel-esir force-pushed the strided_slice_fix branch from f6b646a to 757432a Compare February 3, 2021 23:52

pavel-esir added 7 commits February 5, 2021 14:18

some refactoring

003efb0

fixed extending masks in extractor, fixed licence year and some other…

25da477

… code clearing

corrected a couple of unittests

5cf59a8

fox permute for 5 rank slice and 4 rank inputs/

37fc9c5

WIP

2e01ad4

Added comments

9fd9b2a

fixed StridedSlice in ProposalMutation.py

bb2fdb1

pavel-esir force-pushed the strided_slice_fix branch from 757432a to bb2fdb1 Compare February 5, 2021 11:20

pavel-esir marked this pull request as ready for review February 5, 2021 11:20

pavel-esir requested review from a team, e-nugmanova, rkazants and sadolini February 5, 2021 11:20

pavel-esir added the category: MO Model Optimizer label Feb 5, 2021

pavel-esir added this to the 2021.3 milestone Feb 5, 2021

lazarevevgeny suggested changes Feb 6, 2021

View reviewed changes

lazarevevgeny suggested changes Feb 7, 2021

View reviewed changes

pavel-esir added 2 commits February 11, 2021 23:16

Merge branch 'master' into strided_slice_fix

4425c56

minor corrections

376378a

lazarevevgeny reviewed Feb 13, 2021

View reviewed changes

pavel-esir added 5 commits February 15, 2021 12:09

wider range of unittests added (froze the number)

b4496de

review comments applied

d59be03

Merge remote-tracking branch 'upstream/master' into strided_slice_fix

2115e73

enabled skipped unit-test

d01b0e2

comment corrections

73046d7

pavel-esir requested review from lazarevevgeny and sadolini February 15, 2021 10:49

e-nugmanova reviewed Feb 15, 2021

View reviewed changes

sadolini approved these changes Feb 15, 2021

View reviewed changes

applied review comments: changed op -> type, added some asserts, corr…

a8da55a

…ected comments and other minor corrections

pavel-esir requested a review from e-nugmanova February 15, 2021 15:29

lazarevevgeny reviewed Feb 15, 2021

View reviewed changes

model-optimizer/extensions/middle/StridedSliceNormalizer.py Outdated Show resolved Hide resolved

lazarevevgeny reviewed Feb 15, 2021

View reviewed changes

model-optimizer/extensions/middle/StridedSliceNormalizer.py Outdated Show resolved Hide resolved

lazarevevgeny reviewed Feb 15, 2021

View reviewed changes

model-optimizer/extensions/middle/StridedSliceNormalizer.py Outdated Show resolved Hide resolved

lazarevevgeny approved these changes Feb 15, 2021

View reviewed changes

lazarevevgeny reviewed Feb 15, 2021

View reviewed changes

model-optimizer/extensions/middle/StridedSliceNormalizer.py Outdated Show resolved Hide resolved

e-nugmanova reviewed Feb 15, 2021

View reviewed changes

model-optimizer/extensions/middle/StridedSliceNormalizer.py Show resolved Hide resolved

sorted inputs, updated Supported_Frameworks_Layers.md, some minor

b359de5

pavel-esir requested a review from a team as a code owner February 15, 2021 17:07

rkazants approved these changes Feb 16, 2021

View reviewed changes

lazarevevgeny merged commit 22169a0 into openvinotoolkit:master Feb 16, 2021

e-nugmanova reviewed Feb 16, 2021

View reviewed changes

docs/MO_DG/prepare_model/Supported_Frameworks_Layers.md Show resolved Hide resolved

pavel-esir deleted the strided_slice_fix branch February 28, 2021 18:30

pavel-esir mentioned this pull request Mar 1, 2021

[MO] Fix StridedSlice replacer order and input permutation without strides #4545

Merged

11 tasks

Conversation

pavel-esir commented Feb 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lazarevevgeny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pavel-esir commented Feb 15, 2021

Uh oh!

e-nugmanova left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lazarevevgeny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

likholat commented Feb 17, 2021

Uh oh!

pavel-esir commented Feb 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pavel-esir commented Feb 2, 2021 •

edited

Loading

pavel-esir commented Feb 17, 2021 •

edited

Loading