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] Fix conv_ops_test and remapper_test #39548

Merged
merged 1 commit into from
May 18, 2020
Merged

[INTEL MKL] Fix conv_ops_test and remapper_test #39548

merged 1 commit into from
May 18, 2020

Conversation

gzmkl
Copy link
Contributor

@gzmkl gzmkl commented May 14, 2020

Fix two C++ test failures related to MKL ops.

  1. conv_ops_test // MklConvOp does not support EXPLICIT padding
  2. remapper_test // Fusion of MKL Conv and Mkl FusedBatchNorm is not supported

The fix is to disable the related tests with MKL build.

@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label May 14, 2020
@gbaned gbaned self-assigned this May 14, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation May 14, 2020
@gbaned gbaned requested a review from ezhulenev May 14, 2020 16:25
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer May 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 May 14, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 14, 2020
@tensorflow-copybara tensorflow-copybara merged commit 9b9a2dd into tensorflow:master May 18, 2020
@ezhulenev
Copy link
Member

This broke Windows build

tensorflow/core/kernels/conv_ops_test.cc(1164): error C2121: '#': invalid character: possibly the result of a macro expansion
tensorflow/core/kernels/conv_ops_test.cc(1153): error C2065: 'ifndef': undeclared identifier
tensorflow/core/kernels/conv_ops_test.cc(1153): error C2146: syntax error: missing '>' before identifier 'INTEL_MKL'
tensorflow/core/kernels/conv_ops_test.cc(1153): error C2065: 'endif': undeclared identifier
tensorflow/core/kernels/conv_ops_test.cc(1164): error C2062: type 'unknown-type' unexpected
tensorflow/core/kernels/conv_ops_test.cc(1153): error C2039: 'type': is not a member of '`global namespace''
tensorflow/core/kernels/conv_ops_test.cc(1180): error C2121: '#': invalid character: possibly the result of a macro expansion
tensorflow/core/kernels/conv_ops_test.cc(1169): error C2065: 'ifndef': undeclared identifier
tensorflow/core/kernels/conv_ops_test.cc(1169): error C2146: syntax error: missing '>' before identifier 'INTEL_MKL'
tensorflow/core/kernels/conv_ops_test.cc(1169): error C2065: 'endif': undeclared identifier
tensorflow/core/kernels/conv_ops_test.cc(1180): error C2062: type 'unknown-type' unexpected
tensorflow/core/kernels/conv_ops_test.cc(1169): error C2039: 'type': is not a member of '`global namespace''
tensorflow/core/kernels/conv_ops_test.cc(1181): fatal error C1019: unexpected #else

Any idea what it might be?

tensorflow-copybara pushed a commit that referenced this pull request May 21, 2020
@qlzh727 @tensorflower-gardener
PR #39548: [INTEL MKL] Fix conv_ops_test and remapper_test
b6d6b45 
Imported from GitHub PR #39548

Fix two C++ test failures related to MKL ops.

1. conv_ops_test       // MklConvOp does not support EXPLICIT padding
2. remapper_test      // Fusion of MKL Conv and Mkl FusedBatchNorm is not supported

The fix is to disable the related tests with MKL build.
Copybara import of the project:

--
5d92849 by Guozhong Zhu...

***

PiperOrigin-RevId: 312742653
Change-Id: I0393c00589c3d2bc04965e390c2b2ba249da0432
@gzmkl gzmkl mentioned this pull request Jun 22, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ezhulenev ezhulenev ezhulenev approved these changes

Assignees

@gbaned gbaned

Labels
cla: yes ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Approved by Reviewer
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@gzmkl @ezhulenev @googlebot @kokoro-team @tensorflow-copybara @gbaned