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 check equality between static shape and dynamic shape in test #10879

Merged
merged 2 commits into from
Aug 14, 2018

Conversation

yanboliang
Copy link
Contributor

Summary

At backend_test.py, we would like to check the equality between static keras shape and dynamic shape. However, we actually don't trigger this check by mistake in the existing code. See my inline comments for detail.

Related Issues

PR Overview

  • This PR requires new unit tests [y/n] (make sure tests are included)
  • This PR requires to update the documentation [y/n] (make sure the docs are up-to-date)
  • This PR is backwards compatible [y/n]
  • This PR changes the current API [y/n] (all API changes need to be approved by fchollet)

@@ -128,7 +132,7 @@ def check_single_tensor_operation(function_name, x_shape_or_val, backend_list, *
assert_list_with_ref(z_list, assert_value_with_ref)
else:
assert_list_pairwise(z_list, allclose=assert_value_equality)
assert_list_keras_shape(z_list)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally z is the output of K.eval which is a numpy ndarray usually. It doesn't have _keras_shape attribute, so not trigger the inside check.
The fix is: we pass into two list, one is tensor list, the other is output value by K.eval. We check the equality between tensor._keras_shape and value.shape.

@@ -1144,7 +1144,18 @@ def spatial_2d_padding(x, padding=((1, 1), (1, 1)), data_format=None):
py_slice(left_pad, input_shape[2] + left_pad),
py_slice(None))
y = T.set_subtensor(output[indices], x)
y._keras_shape = output_shape
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The keras shape is not correct, actually the test(backend_test.py/test_spatial_2d_padding) already cover this case, but due to the bug mentioned/fixed in this PR, it doesn't complain.

Copy link
Collaborator

@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 for the fix!

@fchollet fchollet merged commit f99f633 into keras-team:master Aug 14, 2018
@yanboliang yanboliang deleted the test-shape branch August 14, 2018 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants