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

[IENN-NGRAPH]Add support for quant deeplab model #56

Merged
merged 9 commits into from
Nov 2, 2020
Merged

[IENN-NGRAPH]Add support for quant deeplab model #56

merged 9 commits into from
Nov 2, 2020

Conversation

lisa0314
Copy link

@fujunwei @mingmingtasd Please help to review. Thanks!

@lisa0314
Copy link
Author

lisa0314 commented Sep 16, 2020 •
edited
Loading

There are no regression for test cases and examples

  IE-MKLDNN IE-clDNN IE-GNA
baseline pass: 585 fail: 313 pass: 545 fail: 353 pass: 194 fail: 704
current pass: 603 fail: 295 pass: 567 fail: 331 pass: 194 fail: 704
Image Classification IE-clDNN IE-MKLDNN
MobileNet v1(TFLite) Yes Yes
MobileNet v2(TFLite) Yes Yes
SqueezeNet(TFLite) Yes Yes
Inception v3(TFLite) Yes Yes
SqueezeNet(ONNX) Yes Yes
MobileNet v2(ONNX) Yes Yes
SqueezeNet(OpenVINO) Yes Yes
Deeplab 224/257/321/513 Yes Yes

fujunwei
fujunwei reviewed Sep 16, 2020
View reviewed changes
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Show resolved Hide resolved
@fujunwei
Copy link

@lisa0314 please rebase the PR based on latest code, thanks.

@lisa0314
Copy link
Author

lisa0314 commented Oct 9, 2020

@fujunwei rebase done! Please help to review again. Thanks!

fujunwei
fujunwei reviewed Oct 10, 2020
View reviewed changes
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
@lisa0314
Copy link
Author

@fujunwei I have update the PR as your comments. Please help to review again. Thanks!

fujunwei
fujunwei reviewed Oct 10, 2020
View reviewed changes
third_party/ienn/src/ie_compilation.cpp Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
third_party/ienn/src/ie_compilation.cpp Outdated Show resolved Hide resolved
@lisa0314
Copy link
Author

@fujunwei Done! Please help to merge it. Thanks!

@fujunwei
Copy link

LGTM, @huningxin PTAL.

@lisa0314
Copy link
Author

@huningxin PTAL. Thanks!

@fujunwei
Copy link

@lisa0314 Please update the binary including windows and linux and regression checker for test cases and some examples like #58 (comment), thanks.

@huningxin
Copy link

LGTM. Thanks!

@mingmingtasd
Copy link

mingmingtasd commented Oct 13, 2020 •
edited
Loading

On ICL I5 for Windows:
Updated the results as below. There is no regression and deeplab 224 quant model can get good performance now.

  IE-MKLDNN IE-clDNN IE-GNA
baseline pass: 585 fail: 313 pass: 545 fail: 353 pass: 194 fail: 704
current pass: 603 fail: 295 pass: 567 fail: 331 pass: 194 fail: 704
Image Classification IE-clDNN IE-MKLDNN
MobileNet v1(TFLite) Yes Yes
MobileNet v2(TFLite) Yes Yes
SqueezeNet(TFLite) Yes Yes
Inception v3(TFLite) Yes Yes
SqueezeNet(ONNX) Yes Yes
MobileNet v2(ONNX) Yes Yes
SqueezeNet(OpenVINO) Yes Yes
Deeplab 224/513(TFLite) Yes Yes
Semantic Segmentation IE-clDNN IE-MKLDNN
Deeplab 224 Yes Yes
Deeplab 224 Quant Yes Yes
Deeplab 257 Yes Yes
Deeplab 257 Quant Yes Yes
Deeplab 321 Yes Yes
Deeplab 321 Quant Yes Yes
Deeplab 513 Yes Yes
Deeplab 513 Quant Yes Yes

@lisa0314
Copy link
Author

@fujunwei code has bee updated! After this PR merged, ienn could both support NCHW and NHWC layout. Please help to review again! Thanks!

fujunwei
fujunwei reviewed Oct 30, 2020
View reviewed changes
Copy link

@fujunwei fujunwei left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM with one nit. Thanks @lisa0314
Please make sure it can be passed regression checker.

third_party/ienn/src/ie_compilation.cpp Outdated
@@ -195,12 +195,13 @@ int32_t Compilation::Compile() {

OutputsDataMap output_info(network_->getOutputsInfo());
for (auto itr : output_info) {
itr.second->setPrecision(Precision::FP32);
SizeVector dims = itr.second->getTensorDesc().getDims();
Copy link

Choose a reason for hiding this comment

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

Why remove this line and add the unused code?

Copy link
Author

Choose a reason for hiding this comment

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

@fujunwei I remove the itr.second->setPrecision(Precision::FP32) since the output of argmax is int32. I remove the output process code int32 to float in ie_execution.cpp. The unused code has been removed.

mingmingtasd
mingmingtasd reviewed Oct 30, 2020
View reviewed changes
third_party/ienn/src/ie_compilation.cpp Outdated
@@ -588,8 +703,12 @@ int32_t Compilation::AddFullyConnectedV1(const Operation& operation) {
try {
const uint32_t weights_index = operation.inputs[1];
const uint32_t bias_index = operation.inputs[2];
AddConstant(weights_index, true);
AddConstant(bias_index, true);
if (index_op_map_.find(weights_index) == index_op_map_.end()) {
Copy link

Choose a reason for hiding this comment

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

Line 244 has been added, so pls delete it here.

Copy link
Author

Choose a reason for hiding this comment

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

done.

@mingmingtasd mingmingtasd mentioned this pull request Oct 30, 2020
Merged
@lisa0314
Copy link
Author

lisa0314 commented Oct 30, 2020 •
edited
Loading

@fujunwei Regression checker and examples test have been updated for Linux and Windows.

@fujunwei
Copy link

fujunwei commented Nov 2, 2020

Let's merge, thanks @lisa0314 .

@fujunwei fujunwei merged commit 9a1299d into otcshare:webml Nov 2, 2020
fujunwei pushed a commit that referenced this pull request Apr 22, 2022
[VCN] Add billable_service_number for Unenroll request
fd35f0f 
This is used on the server side for logging purposes

(cherry picked from commit 7389124)

Bug: 1317443
Change-Id: Ib782da6dd4a33324381e2ada365dc2e7d92073c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3585070
Reviewed-by: Vinny Persky <vinnypersky@google.com>
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Siyu An <siyua@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#992785}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3595259
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/branch-heads/5005@{#56}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fujunwei fujunwei fujunwei left review comments

@mingmingtasd mingmingtasd mingmingtasd left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@lisa0314 @fujunwei @huningxin @mingmingtasd