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

Add relevant shape check for tf.reshape to prevent crash #46717

Merged

Conversation

yongtang
Copy link
Member

@yongtang yongtang commented Jan 27, 2021

This PR tries to address the issue raised in #46693 where
a shape with large number of elements will cause the
tf.reshape to crash.

This PR adds relevant shape check so that error message can
be returned gracefully.

This PR fixes #46693

This PR also fixes #46699

Signed-off-by: Yong Tang yong.tang.github@outlook.com

@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label Jan 27, 2021
@google-cla google-cla bot added the cla: yes label Jan 27, 2021
@gbaned gbaned self-assigned this Jan 27, 2021
@gbaned gbaned added comp:core issues related to core part of tensorflow prtype:bugfix PR to fix a bug labels Jan 27, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jan 27, 2021
@gbaned gbaned added the awaiting review Pull request awaiting review label Feb 5, 2021
@gbaned gbaned requested a review from hyeygit February 16, 2021 16:24
}
return errors::InvalidArgument("Shape [", msg,
"] has too many elements");
}
Copy link
Member

Choose a reason for hiding this comment

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

The indentation here looks wrong; might be a spaces (the rest of the file) vs. tabs issue?

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Feb 22, 2021
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Feb 22, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 22, 2021
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Feb 23, 2021
@yongtang
Copy link
Member Author

Thanks @allenlavoie for the review. Indeed that was a tab issue. I have updated the PR and fixed it. Please take a look.

Copy link
Member

@allenlavoie allenlavoie left a comment

Choose a reason for hiding this comment

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

Nice fix

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Feb 23, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 23, 2021
@gbaned gbaned removed awaiting review Pull request awaiting review ready to pull PR ready for merge process labels Feb 23, 2021
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Feb 24, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 24, 2021
@allenlavoie
Copy link
Member

The merge isn't working because we have some presubmits which run the test in a 1.x-style Graph+Session. Can you please decorate the test method with "@test_util.run_v2_only"?

This PR tries to address the issue raised in 46693 where
a shape with large number of elements will cause the
tf.reshape to crash.

This PR adds relevant shape check so that error message can
be returned gracefully.

This PR fixes 46693

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Mar 1, 2021
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Mar 1, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 1, 2021
@copybara-service copybara-service bot merged commit fab8fb9 into tensorflow:master Mar 4, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Mar 4, 2021
@yongtang yongtang deleted the 46693-reshape-validation branch March 4, 2021 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:core issues related to core part of tensorflow prtype:bugfix PR to fix a bug ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

tf.keras.backend.constant abortion tf.keras.backend.reshape abortion when shape contain large values
4 participants