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.

Sign up for GitHub

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

Jump to bottom

Wrapped tf.nn.RNNCell* layers are incompatible with tf.keras.layers.Bidirectional #26581

Closed
guillaumekln opened this issue Mar 11, 2019 · 5 comments
Closed

Wrapped tf.nn.RNNCell* layers are incompatible with tf.keras.layers.Bidirectional #26581

guillaumekln opened this issue Mar 11, 2019 · 5 comments
Assignees
@qlzh727
Labels
comp:keras Keras related issues TF 2.0 Issues relating to TensorFlow 2.0 type:bug Bug

Comments

@guillaumekln
Copy link
Contributor

System information

  • Have I written custom code: Yes
  • OS Platform and Distribution: Ubuntu 16.04
  • TensorFlow installed: binary
  • TensorFlow version: 2.0.0.dev20190311
  • Python version: 3.6.6

Describe the current behavior

In the TensorFlow 2.0 preview, the tf.nn.RNNCellDropoutWrapper and tf.nn.RNNCellResidualWrapper wrappers are incompatible with tf.keras.layers.Bidirectional. It raises an "Unknown layer" exception.

Describe the expected behavior

The RNN classes in tf.nn should be compatible with all Keras RNN layers.

Code to reproduce the issue

import tensorflow as tf

cell = tf.keras.layers.LSTMCell(10)
cell = tf.nn.RNNCellResidualWrapper(cell)

rnn = tf.keras.layers.RNN(cell)
rnn = tf.keras.layers.Bidirectional(rnn)

Other info / logs

Traceback (most recent call last):
  File "test/residual_wrapper.py", line 30, in <module>
    rnn = tf.keras.layers.Bidirectional(rnn)
  File "/lib/python3.6/site-packages/tensorflow/python/keras/layers/wrappers.py", line 393, in __init__
    self.backward_layer = layer.__class__.from_config(config)
  File "/lib/python3.6/site-packages/tensorflow/python/keras/layers/recurrent.py", line 920, in from_config
    cell = deserialize_layer(config.pop('cell'), custom_objects=custom_objects)
  File "/lib/python3.6/site-packages/tensorflow/python/keras/layers/serialization.py", line 95, in deserialize
    printable_module_name='layer')
  File "/lib/python3.6/site-packages/tensorflow/python/keras/utils/generic_utils.py", line 181, in deserialize_keras_object
    config, module_objects, custom_objects, printable_module_name)
  File "/lib/python3.6/site-packages/tensorflow/python/keras/utils/generic_utils.py", line 166, in class_and_config_for_serialized_keras_object
    raise ValueError('Unknown ' + printable_module_name + ': ' + class_name)
ValueError: Unknown layer: ResidualWrapperV2

@qlzh727
@qlzh727 qlzh727 self-assigned this Mar 11, 2019
@qlzh727
Copy link
Member

qlzh727 commented Mar 11, 2019

Thanks for reporting the issue, I will send some fix within this week.

@qlzh727 qlzh727 added comp:keras Keras related issues TF 2.0 Issues relating to TensorFlow 2.0 labels Mar 11, 2019
@qlzh727 qlzh727 changed the title tf.nn.RNNCell* layers are incompatible with tf.keras.layers.Bidirectional Wrapped tf.nn.RNNCell* layers are incompatible with tf.keras.layers.Bidirectional Apr 5, 2019
@ruanchong
Copy link

This is weird. Why tf.nn.rnn_cell.* is moved to tf.keras, but ResidualWrapper and DeviceWrapper are not (and they are inheriting new base classes such like ResidualWrapperV2)?

tensorflow-copybara pushed a commit that referenced this issue May 15, 2019
@qlzh727 @tensorflower-gardener
Fix customized cell handling for Bidirectional wrapper.
0667725 
See #26581 for more details.

PiperOrigin-RevId: 248418981
@qlzh727
Copy link
Member

qlzh727 commented May 15, 2019

Sorry for the very late reply. The issue was caused by the serial/deserialization for a non-keras object, and the nn.RNNCell wrappers are not keras object. The commit 0667725 is trying to fix the custom object serialization issue, which allows you to do something like https://gist.github.com/fchollet/9e361d1fe9046b512a2f3e0d31382a96.

We are still discussing the issue internally and see how we will fix this issue.

@qlzh727
Copy link
Member

qlzh727 commented Jun 4, 2019

Sorry for the long wait, this should now by fixed by e62dc43

@qlzh727 qlzh727 closed this as completed Jun 4, 2019
@tensorflow-bot
Copy link

tensorflow-bot bot commented Jun 4, 2019

Are you satisfied with the resolution of your issue?
Yes
No

sleighsoft pushed a commit to sleighsoft/tensorflow that referenced this issue Jun 12, 2019
@qlzh727 @sleighsoft
Fix serial/deserialization issue for rnn wrapper.
778ed3f 
Split the v2 implementation into separate module so that the v1 and v2
class can have same name in the json config.

Fixing for tensorflow#26581.

PiperOrigin-RevId: 251074685
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@qlzh727 qlzh727

Labels
comp:keras Keras related issues TF 2.0 Issues relating to TensorFlow 2.0 type:bug Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@guillaumekln @qlzh727 @ruanchong @jvishnuvardhan