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

Fix all conv layers when filters is 0 #48566

Merged
merged 9 commits into from Apr 20, 2021

Conversation

around-star
Copy link
Contributor

Fix #48470. This PR raises a ValueError for all conv layers upon encountering filters = 0. @fchollet

@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label Apr 16, 2021
@google-cla google-cla bot added the cla: yes label Apr 16, 2021
@gbaned gbaned self-assigned this Apr 19, 2021
@gbaned gbaned added comp:keras Keras related issues prtype:bugfix PR to fix a bug labels Apr 19, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Apr 19, 2021
@gbaned gbaned self-requested a review April 19, 2021 03:32
@around-star
Copy link
Contributor Author

@fchollet can you review this ?

Copy link
Member

@fchollet fchollet 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.

tensorflow/python/keras/layers/convolutional_test.py Outdated Show resolved Hide resolved
PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes Apr 19, 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 Apr 19, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 19, 2021
@google-ml-butler google-ml-butler bot added the kokoro:force-run Tests on submitted change label Apr 19, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 19, 2021
@@ -298,6 +303,11 @@ def test_conv2d_zero_kernel_size(self):
with self.assertRaises(ValueError):
keras.layers.Conv2D(**kwargs)

def test_conv2d_zero_filters(self):
kwargs = {'filters': 0, 'kernel_size': 2}
with self.assertRaisesRegex(ValueError):
Copy link
Contributor

Choose a reason for hiding this comment

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

It has two params self.assertRaisesRegex(ValueError, <str>):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bhack sorry about that. I fixed it.

@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Apr 20, 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 Apr 20, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 20, 2021
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Apr 20, 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 Apr 20, 2021
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Apr 20, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 20, 2021
@around-star
Copy link
Contributor Author

I guess I run into some pylint errors . Solved them. Please review @bhack

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Apr 20, 2021
Copy link
Member

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Apr 20, 2021
@copybara-service copybara-service bot merged commit a057423 into tensorflow:master Apr 20, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Apr 20, 2021
@around-star around-star deleted the patch-2 branch April 20, 2021 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:keras Keras related issues kokoro:force-run Tests on submitted change 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.

Conv2DTranspose crashes with filters=0
6 participants