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

[Intel MKL] support MKL Quantized Matmul With Bias and Requantize Op #33926

Merged
merged 8 commits into from Nov 26, 2019
Merged

[Intel MKL] support MKL Quantized Matmul With Bias and Requantize Op #33926

merged 8 commits into from Nov 26, 2019
+236 −6

Conversation

nammbash
Copy link
Contributor

@nammbash nammbash commented Nov 1, 2019 •
edited

Add Support for MKL QuantizedMatMulWithBiasAndRequantize OP.

@penpornk There is one more PR which is waiting for quite while which needs some help.
#32486 (related to the same model I am working on)

@tensorflow-bot tensorflow-bot bot added the size:M CL Change Size: Medium label Nov 1, 2019
@gbaned gbaned self-assigned this Nov 4, 2019
@gbaned gbaned added the comp:mkl MKL related issues label Nov 4, 2019
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Nov 4, 2019
@gbaned gbaned requested a review from penpornk November 4, 2019 03:59
@nammbash
Copy link
Contributor Author

nammbash commented Nov 5, 2019 •
edited

@penpornk @gbaned. friendly reminder and thank you.
Note: This is "NOT" a part of 2.1.

@gbaned gbaned added the awaiting review Pull request awaiting review label Nov 6, 2019
@nammbash
Copy link
Contributor Author

nammbash commented Nov 8, 2019

@penpornk THank you for the help with 2.1 Release. Just thought I will make a friendly reminder here.

@nammbash
Copy link
Contributor Author

@penpornk @gbaned Any Updates on this please!

penpornk
penpornk requested changes Nov 12, 2019
View reviewed changes
Copy link
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

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

I'm so sorry for my delay! I have a minor comment and we should be good to go.

tensorflow/core/kernels/mkl_qmatmul_op.cc Outdated Show resolved Hide resolved
tensorflow/core/kernels/mkl_qmatmul_op_test.cc Show resolved Hide resolved
PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes Nov 12, 2019
@nammbash @penpornk
Update tensorflow/core/kernels/mkl_qmatmul_op.cc
cd09182 
Yes IP is matmul

Co-Authored-By: Penporn Koanantakool <38085909+penpornk@users.noreply.github.com>
@nammbash
Copy link
Contributor Author

nammbash commented Nov 12, 2019 •
edited

@penpornk done!

penpornk
penpornk approved these changes Nov 12, 2019
View reviewed changes
Copy link
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

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

Thank you again for the PR!
PR #32486 is merged. :)

PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Nov 12, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Nov 12, 2019
@penpornk penpornk removed the awaiting review Pull request awaiting review label Nov 12, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 12, 2019
@nammbash
Copy link
Contributor Author

@gbaned @penpornk friendly ping.
Add Kokoro:force to rerun build? or merge?

@penpornk penpornk added the kokoro:force-run Tests on submitted change label Nov 13, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 13, 2019
penpornk
penpornk requested changes Nov 13, 2019
View reviewed changes
Copy link
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

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

This PR failed the //tensorflow/tools/api/tests:api_compatibility_test in Ubuntu CPU. Please update the API golden files by running:
$ bazel run tensorflow/tools/api/tests:api_compatibility_test -- --update_goldens True

See details in the log

After that, this PR will need to go through API review.

PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Nov 13, 2019
@nammbash
Copy link
Contributor Author

@penpornk Thank you! Update the API Golden files. Let's see what the CI gives us.

penpornk
penpornk reviewed Nov 14, 2019
View reviewed changes
Copy link
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

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

Thank you very much! I'm tagging API review now. Will defer to the API reviewer for explicit PR approval.

@penpornk penpornk added API review API Review kokoro:force-run Tests on submitted change and removed ready to pull PR ready for merge process labels Nov 14, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 14, 2019
@nammbash
Copy link
Contributor Author

@gbaned can you push the review please. :)

@gbaned gbaned requested a review from alextp November 20, 2019 11:38
alextp
alextp approved these changes Nov 25, 2019
View reviewed changes
Copy link
Contributor

@alextp alextp left a comment

Choose a reason for hiding this comment

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

API approval

@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Nov 25, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 25, 2019
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Nov 25, 2019
@tensorflow-bot tensorflow-bot bot added the kokoro:force-run Tests on submitted change label Nov 25, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 25, 2019
tensorflow-copybara pushed a commit that referenced this pull request Nov 26, 2019
@tensorflower-gardener
Merge pull request #33926 from Intel-tensorflow:niroop/MKL_QuantizedM…
60b766d 
…atmulWithRequantizeOp

PiperOrigin-RevId: 282455889
Change-Id: Icc59e3047a333c4bf043c49e51e14ffbb3c3e18c
@tensorflow-copybara tensorflow-copybara merged commit f1b105b into tensorflow:master Nov 26, 2019
PR Queue automation moved this from Approved by Reviewer to Merged Nov 26, 2019
@nammbash nammbash deleted the niroop/MKL_QuantizedMatmulWithRequantizeOp branch November 26, 2019 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alextp alextp alextp approved these changes

@penpornk penpornk penpornk approved these changes

Assignees

@gbaned gbaned

Labels
API review API Review cla: yes comp:mkl MKL related issues ready to pull PR ready for merge process size:M CL Change Size: Medium
Projects
PR Queue
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@nammbash @alextp @penpornk @googlebot @kokoro-team @tensorflow-copybara @gbaned