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

Simplify shape inference in Keras resize_images #47011

Merged
merged 2 commits into from Feb 25, 2021

Conversation

lgeiger
Copy link
Contributor

@lgeiger lgeiger commented Feb 8, 2021

This PR simplifies shape inference of keras.backend.resize_images, since the shape of the output tensor is already correctly set in image_ops.resize_images_v2:

images.set_shape([None, new_height_const, new_width_const, None])

This also removes the need for the tf.shape op in cases where the spatial shape of the input tensor is fully defined which fixes #25086.

@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label Feb 8, 2021
@google-cla google-cla bot added the cla: yes label Feb 8, 2021
@gbaned gbaned added comp:keras Keras related issues prtype:bugfix PR to fix a bug labels Feb 9, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Feb 9, 2021
@gbaned gbaned self-assigned this Feb 9, 2021
@gbaned gbaned self-requested a review February 9, 2021 04:33
@gbaned gbaned added the awaiting review Pull request awaiting review label Feb 16, 2021
@mattdangerw
Copy link
Member

Please add a unit test!

@lgeiger
Copy link
Contributor Author

lgeiger commented Feb 18, 2021

@mattdangerw the correct output shape computations should already be covered in the following test cases

def test_resize_images(self):
height_factor = 2
width_factor = 2
data_format = 'channels_last'
x = backend.variable(np.ones((1, 2, 2, 3)))
y = backend.resize_images(x, height_factor, width_factor, data_format)
self.assertEqual(y.shape.as_list(), [1, 4, 4, 3])
data_format = 'channels_first'
x = backend.variable(np.ones((1, 3, 2, 2)))
y = backend.resize_images(x, height_factor, width_factor, data_format)
self.assertEqual(y.shape.as_list(), [1, 3, 4, 4])
# Invalid use:
with self.assertRaises(ValueError):
backend.resize_images(
x, height_factor, width_factor, data_format='unknown')

But let me know if there is still a case I am missing.

@gbaned gbaned removed the awaiting review Pull request awaiting review label Feb 19, 2021
@mattdangerw
Copy link
Member

Look like we are missing a case for a partially defined shape to me. Something that exercises the else block in your change.

@lgeiger
Copy link
Contributor Author

lgeiger commented Feb 22, 2021

@mattdangerw Sorry about that, I added a test case for dynamic input shapes.

Copy link
Member

@mattdangerw mattdangerw left a comment

Choose a reason for hiding this comment

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

Thanks! lgtm

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Feb 24, 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
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Feb 25, 2021
@copybara-service copybara-service bot merged commit a2cbfe5 into tensorflow:master Feb 25, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Feb 25, 2021
@lgeiger lgeiger deleted the resize-image-shapes branch February 25, 2021 23:54
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 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
4 participants