EderSantana / seya

Bringing up some extra Cosmo to Keras.
Other
377 stars 103 forks source link

BRNN examles fails with Keras 1.0.1 #28

Open sun9700 opened 8 years ago

sun9700 commented 8 years ago

I tired seya with the lasted keras(1.0.1), unfortunately, Seya fails with the following outputs:

/usr/bin/python2.7 /home/sun/seya/examples/imdb_brnn.py Using TensorFlow backend. Loading data... 20000 train sequences 5000 test sequences Pad sequences (samples x time) X_train shape: (20000, 100) X_test shape: (5000, 100) Build model... Traceback (most recent call last): File "/home/sun/seya/examples/imdb_brnn.py", line 59, in model.add(brnn) # try using another Bidirectional RNN inside the Bidirectional RNN. Inception meets callback hell. File "/usr/local/lib/python2.7/dist-packages/keras/models.py", line 139, in add output_tensor = layer(self.outputs[0]) File "/usr/local/lib/python2.7/dist-packages/keras/engine/topology.py", line 458, in call self.build(input_shapes[0]) TypeError: build() takes exactly 1 argument (2 given)

Process finished with exit code 1

EderSantana commented 8 years ago

seya is not ready for keras-1 yet, sorry!

omarcr commented 8 years ago

is this git still going to be maintained for newest versions of Keras?

EderSantana commented 8 years ago

yeah... I'm just finishing something on reinforcement learning and then I'll come back to this. But François told me adapting layers from keras 0.3.3 to keras 1 should be simple. If you guys want to try to adapt something and make a PR I'll be glad to review the code

osh commented 8 years ago

would definitely love to get attention.py working on K1.0.1 :-/ i'm poking at it but no love yet -

EderSantana commented 8 years ago

it seems that the main idea is to move the get_output to __call__. I hope this is not super urgent for you T_T sorry!

omarcr commented 8 years ago

I would also like to use the spatial transformer in a project of mine. Since this is your code Eder I think it would be faster if you update. We'll wait for you :)

EderSantana commented 8 years ago

btw, I started translating seya to keras1. SpatialTransformer worked for me (with Theano): https://github.com/EderSantana/seya/blob/keras1/seya/layers/attention.py

xypan1232 commented 7 years ago

Bidirectional RNN also gives me the same error, Is the seya ready for keras-1 now? Thanks

EderSantana commented 7 years ago

@xypan1232 not really we are translating one layer at a time when necessity arrives... But I'd check keras itself for bidirectional RNNs. All the RNN layers now have go_backwards option

StuartFarmer commented 7 years ago

@EderSantana What's the process you're going through to convert? I'm not familiar with the old Keras (started using it at 0.9 / 1.0 I believe) but want to use some of the non-implemented layers in this library. Can I make the changes and commit a PR?

EderSantana commented 7 years ago

@StuartFarmer sure, make a PR to the keras1 branch

StuartFarmer commented 7 years ago

@EderSantana Is there a specific commit I should look at for diffs to gain an understanding of what I need to do? I can probably poke around and figure it out, but if you could provide guidance, I could submit a better PR, I believe.

EderSantana commented 7 years ago

sorry, I don't get what you mean... are you writing a new BRNN that works on keras1, right? as long as your new code works on keras-1 and runs the example script, it should be good.

StuartFarmer commented 7 years ago

Right. I'm wondering if you know the specific changes between the Keras 0.3.3 API and the Keras 1.0 API that I should be looking for.

EderSantana commented 7 years ago

oh, yeah! checkout the new functional API changes: https://github.com/fchollet/keras/blob/master/docs/templates/getting-started/functional-api-guide.md

StuartFarmer commented 7 years ago

Awesome! Seems easy enough :) I'll submit some PRs soon.