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

Allow to reshape unknown tensors. #7960

Merged
merged 2 commits into from
Sep 27, 2017

Conversation

hgaiser
Copy link
Contributor

@hgaiser hgaiser commented Sep 22, 2017

The current implementation of keras.layers.Reshape does not allow the reshaping of tensors of unknown shape (ie. (None, None, 3) for any arbitrary image). Strangely enough, the underlying code of tensorflow.reshape does allow this kind of reshape (and already handles -1 in the target shape), I'm not sure about other backends.

Motivation
The following code shows what this PR fixes:

import keras

inputs = keras.layers.Input((None, None, 3))
outputs = keras.layers.Reshape((-1, 1))(inputs)

model = keras.models.Model(inputs=inputs, outputs=outputs)

print("Success!")

Without this PR the result is:

Using TensorFlow backend.
Traceback (most recent call last):
  File "test.py", line 4, in <module>
    outputs = keras.layers.Reshape((-1, 1))(inputs)
  File "/home/hgaiser/.local/lib/python3.6/site-packages/Keras-2.0.8-py3.6.egg/keras/engine/topology.py", line 602, in __call__
  File "/home/hgaiser/.local/lib/python3.6/site-packages/Keras-2.0.8-py3.6.egg/keras/layers/core.py", line 391, in call
  File "/home/hgaiser/.local/lib/python3.6/site-packages/Keras-2.0.8-py3.6.egg/keras/layers/core.py", line 376, in compute_output_shape
  File "/home/hgaiser/.local/lib/python3.6/site-packages/Keras-2.0.8-py3.6.egg/keras/layers/core.py", line 364, in _fix_unknown_dimension
  File "/home/hgaiser/.local/lib/python3.6/site-packages/numpy/core/fromnumeric.py", line 2518, in prod
    out=out, **kwargs)
  File "/home/hgaiser/.local/lib/python3.6/site-packages/numpy/core/_methods.py", line 35, in _prod
    return umr_prod(a, axis, dtype, out, keepdims)
TypeError: int() argument must be a string, a bytes-like object or a number, not 'NoneType'

With this PR the result is:

Using TensorFlow backend.
Success!

ps. my current workaround is:

outputs = keras.layers.Lambda(lambda x: keras.backend.reshape(x,
 (keras.backend.shape(x)[0],) + (-1, 1)))(inputs)

@fchollet
Copy link
Member

I believe the current design was due to previous limitations with TF.

Tests are currently failing. Also, you should add a unit test demonstrating the limitation that is being lifted.

@hgaiser hgaiser force-pushed the reshape-unknown branch 3 times, most recently from 0d994fe to 281723f Compare September 23, 2017 13:44
return (input_shape[0],) + self._fix_unknown_dimension(
input_shape[1:], self.target_shape)
if None in input_shape[1:]:
return (input_shape[0],) + tuple(s if s != -1 else None for s in self.target_shape)
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment describing what this does / why do it, and make this line shorter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

if None in input_shape[1:]:
return (input_shape[0],) + tuple(s if s != -1 else None for s in self.target_shape)
else:
return (input_shape[0],) + self._fix_unknown_dimension(
Copy link
Member

Choose a reason for hiding this comment

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

Since this function is only called once, consider in-lining it (not saying you should do it, but consider whether it would improve the code if you did it -- maybe it would).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had considered that, but I figured keeping it separate would be better. That way it can potentially be used by layers that subclass Reshape (or other code in general). In addition it is more in line with the "separation of concerns" philosophy.

@hgaiser
Copy link
Contributor Author

hgaiser commented Sep 25, 2017

Updated PR as requested.

@fchollet
Copy link
Member

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

@fchollet fchollet merged commit 408de9b into keras-team:master Sep 27, 2017
ozabluda pushed a commit to ozabluda/keras that referenced this pull request Oct 2, 2017
* Allow to reshape unknown tensors.

* Add test for reshaping unknown shapes.
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