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

Reshape-able SliceConverter #3198

Merged
merged 31 commits into from
Dec 1, 2020
Merged

Reshape-able SliceConverter #3198

merged 31 commits into from
Dec 1, 2020

Conversation

yekruglov
Copy link
Contributor

@yekruglov yekruglov commented Nov 18, 2020 •
edited
Loading

Description: Reshape-able convert Slice to StridedSlice

JIRA: 41517

Code:

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

Validation:

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

Documentation:

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

@yekruglov
initial commit
315a309
@yekruglov
add cast
cab3992
@yekruglov
data type fix
c5fd06a
@yekruglov
added tests
d288f98
@yekruglov
added test without axes and steps
d1107b8
@yekruglov
Merge remote-tracking branch 'upstream/master' into ykruglov/reshape/…
1233d05 
…SliceConverter
@yekruglov
remove redundant imports
469ad55
@yekruglov
discussions resolving
ddc4df1
@yekruglov
Merge remote-tracking branch 'upstream/master' into ykruglov/reshape/…
d4ec7de 
…SliceConverter
@yekruglov
Add cast to TFSliceToSlice
f68f06b
@yekruglov
layer tests fix
b30f952
@yekruglov
Merge remote-tracking branch 'upstream/master' into ykruglov/reshape/…
51643f3 
…SliceConverter
@yekruglov
update unittest
2de45f9
@yekruglov
Merge remote-tracking branch 'upstream/master' into ykruglov/reshape/…
4c52c6b 
…SliceConverter
@yekruglov
rework transformation
6045113
@yekruglov
Merge remote-tracking branch 'upstream/master' into ykruglov/reshape/…
ac82b30 
…SliceConverter
@yekruglov
added clamp
30ae90f
@yekruglov
move broadcast
9871095
@yekruglov
update unittests
30bb779
@yekruglov yekruglov added the category: MO Model Optimizer label Nov 18, 2020
@yekruglov yekruglov added this to the 2021.3 milestone Nov 18, 2020
@yekruglov yekruglov requested a review from a team November 18, 2020 13:13
@yekruglov yekruglov self-assigned this Nov 18, 2020
@lazarevevgeny
Copy link
Contributor

@yekruglov , update the PR description according to template.

@yekruglov yekruglov changed the title ykruglov/reshape/SliceConverter Reshape-able SliceConverter Nov 20, 2020
@lazarevevgeny lazarevevgeny self-requested a review November 25, 2020 07:25
lazarevevgeny
lazarevevgeny reviewed Nov 30, 2020
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.

In general looks good. But need to add some comments describing what happens in the transformation and why.

model-optimizer/extensions/middle/SliceConverter.py Show resolved Hide resolved
model-optimizer/extensions/middle/SliceConverter.py Show resolved Hide resolved
jane-intel
jane-intel reviewed Nov 30, 2020
View reviewed changes
Copy link
Contributor

@jane-intel jane-intel left a comment

Choose a reason for hiding this comment

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

Comments are the same as @lazarevevgeny just left
In general looks good

@yekruglov
Copy link
Contributor Author

Do we need more comments in this transformation?

@lazarevevgeny
Copy link
Contributor

Comments are the same as @lazarevevgeny just left
In general looks good

Looks good to me. One more thing. Please, re-run layer tests to make sure that none of them are broken.

@yekruglov
Copy link
Contributor Author

Comments are the same as @lazarevevgeny just left
In general looks good

Looks good to me. One more thing. Please, re-run layer tests to make sure that none of them are broken.

Tests passed

@lazarevevgeny lazarevevgeny merged commit ce1ec9e into openvinotoolkit:master Dec 1, 2020
evolosen pushed a commit to evolosen/openvino that referenced this pull request Dec 3, 2020
@yekruglov @evolosen
Reshape-able SliceConverter (openvinotoolkit#3198)
50a67a0 
* initial commit

* add cast

* data type fix

* added tests

* added test without axes and steps

* remove redundant imports

* discussions resolving

* Add cast to TFSliceToSlice

* layer tests fix

* update unittest

* rework transformation

* added clamp

* move broadcast

* update unittests

* failed e2e fix

* added comment

* little fixes

* comments update
@yekruglov yekruglov deleted the ykruglov/reshape/SliceConverter branch December 7, 2020 09:57
mryzhov pushed a commit to mryzhov/openvino that referenced this pull request Dec 11, 2020
@yekruglov @mryzhov
Reshape-able SliceConverter (openvinotoolkit#3198)
2399041 
* initial commit

* add cast

* data type fix

* added tests

* added test without axes and steps

* remove redundant imports

* discussions resolving

* Add cast to TFSliceToSlice

* layer tests fix

* update unittest

* rework transformation

* added clamp

* move broadcast

* update unittests

* failed e2e fix

* added comment

* little fixes

* comments update
mryzhov pushed a commit to mryzhov/openvino that referenced this pull request Dec 16, 2020
@yekruglov @mryzhov
Reshape-able SliceConverter (openvinotoolkit#3198)
f001f40 
* initial commit

* add cast

* data type fix

* added tests

* added test without axes and steps

* remove redundant imports

* discussions resolving

* Add cast to TFSliceToSlice

* layer tests fix

* update unittest

* rework transformation

* added clamp

* move broadcast

* update unittests

* failed e2e fix

* added comment

* little fixes

* comments update
mryzhov pushed a commit to mryzhov/openvino that referenced this pull request Jan 14, 2021
@yekruglov @mryzhov
Reshape-able SliceConverter (openvinotoolkit#3198)
3616bd4 
* initial commit

* add cast

* data type fix

* added tests

* added test without axes and steps

* remove redundant imports

* discussions resolving

* Add cast to TFSliceToSlice

* layer tests fix

* update unittest

* rework transformation

* added clamp

* move broadcast

* update unittests

* failed e2e fix

* added comment

* little fixes

* comments update
jiwaszki pushed a commit to akuporos/openvino that referenced this pull request Jan 15, 2021
@yekruglov
Reshape-able SliceConverter (openvinotoolkit#3198)
6c211ec 
* initial commit

* add cast

* data type fix

* added tests

* added test without axes and steps

* remove redundant imports

* discussions resolving

* Add cast to TFSliceToSlice

* layer tests fix

* update unittest

* rework transformation

* added clamp

* move broadcast

* update unittests

* failed e2e fix

* added comment

* little fixes

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

@lazarevevgeny lazarevevgeny lazarevevgeny approved these changes

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

Assignees

@yekruglov yekruglov

Labels
category: MO Model Optimizer
Projects
None yet
Milestone
2021.3
Development

Successfully merging this pull request may close these issues.
3 participants
@yekruglov @lazarevevgeny @jane-intel