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

[Grappler] Fix bug in UpdateSqueezeDims #41399

Merged

Conversation

benbarsdell
Copy link
Contributor

The dim logic in generic_layout_transposer's SqueezeTransposer was incorrect, which caused two problems:

  • Negative dims were transformed incorrectly.
  • The dim check incorrectly failed when dim = rank - 1 (which can occur when transforming NCHW to NHWC).

This PR fixes the dim logic and adds tests to check these cases.

cc @nluehr

- The dim logic in generic_layout_transposer's SqueezeTransposer was
  incorrect, which caused two problems:
-   Negative dims were transformed incorrectly.
-   The dim check incorrectly failed when dim = rank - 1 (which can
    occur when transforming NCHW to NHWC).
- Fixes the dim logic and adds tests to check these cases.
@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label Jul 15, 2020
@gbaned gbaned self-assigned this Jul 15, 2020
@gbaned gbaned added the comp:grappler Grappler related issues label Jul 15, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jul 15, 2020
@gbaned gbaned requested a review from rmlarsen July 15, 2020 04:57
@gbaned gbaned added the awaiting review Pull request awaiting review label Jul 17, 2020
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Jul 22, 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 22, 2020
@rmlarsen
Copy link
Member

@benbarsdell thank you for the fix!

@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 22, 2020
@gbaned gbaned removed the awaiting review Pull request awaiting review label Jul 23, 2020
@tensorflow-copybara tensorflow-copybara merged commit 80b11c9 into tensorflow:master Jul 23, 2020
PR Queue automation moved this from Approved by Reviewer to Merged Jul 23, 2020
@benbarsdell benbarsdell deleted the update-squeeze-dims-bug branch July 27, 2020 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:grappler Grappler 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