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

GatherElements: support Gather+GatherElements mode #4220

Merged
merged 1 commit into from
Feb 16, 2021
Merged

GatherElements: support Gather+GatherElements mode #4220

merged 1 commit into from
Feb 16, 2021

Conversation

andrejsokolov
Copy link
Contributor

Support (Gather+GatherElements) optimization for GatherElements layer.
Includes PR: #4140

@andrejsokolov andrejsokolov requested a review from a team February 8, 2021 11:35
@andrejsokolov andrejsokolov requested a review from a team as a code owner February 8, 2021 11:35
@andrejsokolov andrejsokolov requested review from a team February 8, 2021 11:35
@andrejsokolov andrejsokolov assigned ghost Feb 8, 2021
@andrejsokolov andrejsokolov added this to the 2021.3 milestone Feb 8, 2021
@openvino-pushbot openvino-pushbot added category: IE Tests OpenVINO Test: plugins and common category: Core OpenVINO Core (aka ngraph) labels Feb 8, 2021
@GlebKazantaev
Copy link
Contributor

As this PR depends on #4140 which must be merged before this PR I set DO NOT MERGE label for now.

@andrejsokolov
Copy link
Contributor Author

@AndrewBakalinIntel, @elatkin, please take a look (the last commit only).

@andrejsokolov
Copy link
Contributor Author

As this PR depends on #4140 which must be merged before this PR I set DO NOT MERGE label for now.

ok, thank you.

ghost
ghost reviewed Feb 8, 2021
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

In the last state of my PR I have the functional test disabled, pls enable it in your PR:
https://github.com/openvinotoolkit/openvino/pull/4140/files#diff-96c22829f67b8ca6f51e6b160ac5baeb859cc4c41a52fb0fa8f160d6dcbdf7bf

inference-engine/src/vpu/graph_transformer/src/stages/gather_elements.cpp Outdated Show resolved Hide resolved
inference-engine/src/vpu/graph_transformer/src/stages/gather_elements.cpp Outdated Show resolved Hide resolved
@andrejsokolov andrejsokolov force-pushed the vpu/andreyso/gather_elements branch from 4f20f4f to 2e878c1 Compare February 8, 2021 19:02
@andrejsokolov andrejsokolov requested a review from a team February 8, 2021 19:02
@andrejsokolov andrejsokolov force-pushed the vpu/andreyso/gather_elements branch 2 times, most recently from a63d744 to 82de798 Compare February 9, 2021 07:08
@andrejsokolov
Copy link
Contributor Author

In the last state of my PR I have the functional test disabled, pls enable it in your PR:
https://github.com/openvinotoolkit/openvino/pull/4140/files#diff-96c22829f67b8ca6f51e6b160ac5baeb859cc4c41a52fb0fa8f160d6dcbdf7bf

Enabled the tests, but for one DSR test, had to reduce the tensor size (Some problem during the sending tensor to device, just before the layer runs)

@ghost
Copy link

ghost commented Feb 9, 2021 •
edited by ghost
Loading

PR mentioned above has been merged, please rebase on top of the latest master.
BTW, the PR with firmware update has been recently merged: #4178
You probably need to rebuild the firmware as well.

@andrejsokolov andrejsokolov force-pushed the vpu/andreyso/gather_elements branch from 82de798 to 510f533 Compare February 9, 2021 14:30
@ghost ghost removed the DO NOT MERGE label Feb 10, 2021
@GlebKazantaev GlebKazantaev removed the request for review from a team February 10, 2021 15:51
@andrejsokolov andrejsokolov force-pushed the vpu/andreyso/gather_elements branch from 510f533 to 0fd4c47 Compare February 11, 2021 07:53
@andrejsokolov andrejsokolov force-pushed the vpu/andreyso/gather_elements branch 2 times, most recently from 90afef4 to 0a72021 Compare February 11, 2021 18:37
@andrejsokolov andrejsokolov force-pushed the vpu/andreyso/gather_elements branch from 0a72021 to fd827a3 Compare February 12, 2021 05:16
@Maxim-Doronin
Copy link
Contributor

@andrejsokolov, we have just merged another fw. Please, resolve conflicts. Sorry for inconvenience

@Maxim-Doronin Maxim-Doronin assigned andrejsokolov and unassigned ghost Feb 12, 2021
@andrejsokolov andrejsokolov force-pushed the vpu/andreyso/gather_elements branch 2 times, most recently from b6756a3 to 0fc06e3 Compare February 15, 2021 06:39
Maxim-Doronin
Maxim-Doronin requested changes Feb 15, 2021
View reviewed changes
inference-engine/tests/functional/plugin/myriad/shared_tests_instances/skip_tests_config.cpp Outdated
// TODO: Issue 48183
R"(.*CTCGreedyDecoderSeqLen.*?\(1.1.1\).*)",
R"(.*CTCGreedyDecoderSeqLen.*?\(0.1.1\).*)",
Copy link
Contributor

Choose a reason for hiding this comment

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

@andrejsokolov, why did it changed? 1.1.1 topology hangs. Please, abort the test build and revert this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Thank you!

@andrejsokolov andrejsokolov force-pushed the vpu/andreyso/gather_elements branch 3 times, most recently from 521e939 to 2856d62 Compare February 16, 2021 03:38
@andrejsokolov andrejsokolov force-pushed the vpu/andreyso/gather_elements branch from 2856d62 to 0c3319e Compare February 16, 2021 07:26
@andrejsokolov
Copy link
Contributor Author

@Maxim-Doronin, could we merge the PR?

@Maxim-Doronin Maxim-Doronin merged commit 8278e39 into openvinotoolkit:master Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Maxim-Doronin Maxim-Doronin Maxim-Doronin approved these changes

Assignees

@Maxim-Doronin Maxim-Doronin

@andrejsokolov andrejsokolov

Labels
category: Core OpenVINO Core (aka ngraph) category: IE Tests OpenVINO Test: plugins and common
Projects
None yet
Milestone
2021.3
Development

Successfully merging this pull request may close these issues.
4 participants
@andrejsokolov @GlebKazantaev @Maxim-Doronin @openvino-pushbot