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.

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

[Intel MKL] Adding Quantized Convolution (Int8) - Part 3 #21465

Merged
merged 3 commits into from
Oct 16, 2018

Conversation

mahmoud-abuzaina
Copy link
Contributor

@mahmoud-abuzaina mahmoud-abuzaina commented Aug 8, 2018

This PR is part 3 of four PRs that add quantized version of convolution using MKL-DNN. This part adds changes that are required to quantize a graph and it enables fusions of MKL quantized convolution.

Please note that the below PRs replace this one #19425
Part 1: #21483
Part 2: #21456
Part 3: #21465
Part 4: #21466

@dspinxd
Copy link

dspinxd commented Aug 8, 2018 via email

@tensorflowbutler
Copy link
Member

Nagging Reviewer @tatianashp, @raghuraman-k: You have been added as a reviewer to this pull request. Please add your review or reassign. It has been 50 days with no activity and the awaiting review label has been applied.

Copy link
Contributor

@raghuraman-k raghuraman-k left a comment

Choose a reason for hiding this comment

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

Please remove quantize_graph from this PR. It will be best to host it separately.

@tensorflowbutler tensorflowbutler removed the awaiting review Pull request awaiting review label Oct 3, 2018
@mdfaijul
Copy link
Contributor

mdfaijul commented Oct 4, 2018

@raghuraman-k @drpngx removed quantize_graph.py

@drpngx drpngx added the kokoro:force-run Tests on submitted change label Oct 4, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Oct 4, 2018
@mdfaijul
Copy link
Contributor

mdfaijul commented Oct 9, 2018

@drpngx Tests looks fine. Ready to go?

@tatianashp
Copy link
Member

@suharshs Can you take a look? Is this PR still relevant?

suharshs
suharshs previously approved these changes Oct 15, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Oct 15, 2018
@drpngx drpngx added the ready to pull PR ready for merge process label Oct 15, 2018
@drpngx
Copy link
Contributor

drpngx commented Oct 16, 2018

@mahmoud-abuzaina please return an error (invalid argument) for when the tensor parsing fails and not implemented if the dtype is wrong:

CHECK(float_tensor.FromProto(float_tensor_proto));
CHECK_EQ(float_tensor.dtype(), DT_FLOAT);

We do not allow CHECK in new code.

@drpngx drpngx added stat:awaiting response Status - Awaiting response from author and removed ready to pull PR ready for merge process labels Oct 16, 2018
@mdfaijul
Copy link
Contributor

@drpngx Removed CHECK and CHECK_EQ, added errors::InvalidArgument and errors::Unimplemented. Could you please check?

@drpngx drpngx added ready to pull PR ready for merge process and removed stat:awaiting response Status - Awaiting response from author labels Oct 16, 2018
@tensorflow-copybara tensorflow-copybara merged commit 0326792 into tensorflow:master Oct 16, 2018
tensorflow-copybara pushed a commit that referenced this pull request Oct 16, 2018
@mdfaijul mdfaijul deleted the int8-part3 branch February 18, 2020 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet