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] Pad+Conv fusion for bf16 #41010

Merged

Conversation

ShengYang1
Copy link
Contributor

This PR enables pad+conv fusion for bf16 and related test case.

@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label Jul 2, 2020
@gbaned gbaned self-assigned this Jul 2, 2020
@gbaned gbaned added the comp:mkl MKL related issues label Jul 2, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jul 2, 2020
@gbaned gbaned requested a review from penpornk July 2, 2020 03:56
@gbaned gbaned added the awaiting review Pull request awaiting review label Jul 9, 2020
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 for the PR and for refactoring!

const int image_batch_count = 1;
Tensor image(DT_FLOAT, {image_batch_count, image_height, image_width, depth});
test::FillValues<float>(&image, {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12});
TYPED_TEST_CASE_P(FusedPadConvOpTest);
Copy link
Member

Choose a reason for hiding this comment

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

This is deprecated. Please use TYPED_TEST_SUITE_P instead.

Suggested change
TYPED_TEST_CASE_P(FusedPadConvOpTest);
TYPED_TEST_SUITE_P(FusedPadConvOpTest);

const int image_batch_count = 1;
Tensor image(DT_FLOAT, {image_batch_count, depth, image_height, image_width});
test::FillValues<float>(&image, {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12});
REGISTER_TYPED_TEST_CASE_P(FusedPadConvOpTest, PaddingConvTest,
Copy link
Member

Choose a reason for hiding this comment

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

Also deprecated.

Suggested change
REGISTER_TYPED_TEST_CASE_P(FusedPadConvOpTest, PaddingConvTest,
REGISTER_TYPED_TEST_SUITE_P(FusedPadConvOpTest, PaddingConvTest,

#else
using FusedPadConvDataTypes = ::testing::Types<float>;
#endif
INSTANTIATE_TYPED_TEST_CASE_P(Test, FusedPadConvOpTest, FusedPadConvDataTypes);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
INSTANTIATE_TYPED_TEST_CASE_P(Test, FusedPadConvOpTest, FusedPadConvDataTypes);
INSTANTIATE_TYPED_TEST_SUITE_P(Test, FusedPadConvOpTest, FusedPadConvDataTypes);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for review. All TYPED_TEST_CASE_P in this file have been replaced

PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes Jul 14, 2020
@penpornk penpornk removed the awaiting review Pull request awaiting review label Jul 14, 2020
@gbaned gbaned requested a review from penpornk July 14, 2020 15:45
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 for the quick response!

PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Jul 14, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jul 14, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 14, 2020
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Jul 14, 2020
@tensorflow-copybara tensorflow-copybara merged commit 131fc27 into tensorflow:master Jul 14, 2020
PR Queue automation moved this from Approved by Reviewer to Merged Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

None yet

6 participants