philipperemy / cond_rnn

Conditional RNNs for Tensorflow / Keras.
MIT License
225 stars 32 forks source link

Implementation as Keras Layer #5

Closed larsupb closed 4 years ago

larsupb commented 4 years ago

Hi, thank you very much for sharing this code. Im wondering if one could implement the Conditional RNN as a true keras layer (inherited from tf.keras.layers.Layer) so it could be used in a keras model.

This would help peoples which are not familar with tensorflow low level api.

Lars

philipperemy commented 4 years ago

https://github.com/philipperemy/cond_rnn/commit/be1e97fa74ee06ef44e6dec1de8eb662b63004b9

Hey! Thanks for the feedback! Done in the latest master. Also cond-rnn>=2.0 has it implemented.

pepper-jk commented 4 years ago

So in theory one should be able to do something like this:

model = tf.keras.Sequential()
# ...
model.add(cond_rnn.ConditionalRNN(units=rrn_units, cell="LSTM"))
# ...
model.summary()

Correct?

But I get an error when I do that.

Traceback (most recent call last):
  File "classifier.py", line 153, in <module>
    main(sys.argv[1:])
  File "classifier.py", line 138, in main
    model = build_model(NN, padding_value, masking_value, max_seq_len, n_features, rrn_units)
  File "classifier.py", line 27, in build_model
    model.add(rnn) # ([input_shape, 1])
  File "/home/pepper-jk/code/code_bachelor_tud/venv/lib/python3.8/site-packages/tensorflow/python/training/tracking/base.py", line 456, in _method_wrapper
    result = method(self, *args, **kwargs)
  File "/home/pepper-jk/code/code_bachelor_tud/venv/lib/python3.8/site-packages/tensorflow/python/keras/engine/sequential.py", line 213, in add
    output_tensor = layer(self.outputs[0])
  File "/home/pepper-jk/code/code_bachelor_tud/venv/lib/python3.8/site-packages/cond_rnn/cond_rnn.py", line 74, in __call__
    assert isinstance(inputs, list) and len(inputs) >= 2
AssertionError

I don't get why __call__ is even called when initializing the layer and adding it to the model.

I also don't get how I add the input_shape to a ConditinalRNN Layer.

Without conditions I used:

model = tf.keras.Sequential()
# ...
model.add(tf.keras.layers.LSTM(rrn_units, input_shape=input_shape))
# ...
model.summary()

I assumed I could do something like this:

model.add(cond_rnn.ConditionalRNN(units=rrn_units, cell="LSTM", input_shape=input_shape))

or this:

model.add(cond_rnn.ConditionalRNN(units=rrn_units, cell="LSTM", kargs=(input_shape)))

or based on the README:

model.add(cond_rnn.ConditionalRNN(units=rrn_units, cell="LSTM")([input_shape, cond_shape]))

But it seems that I need to provide tensors? Like so:

model.add(cond_rnn.ConditionalRNN(units=rrn_units, cell="LSTM")([input, cond]))

Which besides not working either does not even make sense to me.

What am I doing wrong?

philipperemy commented 4 years ago

@pepper-jk yes in theory you should be able to use Sequential on them but the layer takes two arguments as parameters. In sequential each layer has only one input. This implementation is a bit more on the TF side rather than on the Keras side. I don't have much time at the moment to do the change but if you feel like you can do, just go for it!

ninamwa commented 4 years ago

Hi. Is there done any more work regarding this? I would also like to use the ConditionalRNN layer as a part of my Keras Sequential model :-)

pepper-jk commented 4 years ago

During my thesis I did not have the time, I just used the layer as suggested by the examples. And afterwards I totally forgot about it, sorry.

But I also am not sure how one could implement this, as the number of parameters is different. Maybe there is a misc parameter that could be used for the conditional input? Or should the inputs just be wrapped in a tuple and unpacked internally? Would that conflict with the basic Sequential-Layer class, as the dimensions of targets and inputs must be the same?

This is just off the top of my head, sorry if I missed anything.