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

Same cell for word and sentence level #24

Open
ksingla025 opened this issue Sep 26, 2018 · 3 comments
Open

Same cell for word and sentence level #24

ksingla025 opened this issue Sep 26, 2018 · 3 comments

Comments

@ksingla025
Copy link

In worker.py looking at lines 70-80, it seems you are using the same cell for word and sentence level, but it should be a different lstm cell

@ematvey
Copy link
Owner

ematvey commented Sep 26, 2018

AFAIR Cell is/was a template/factory, not a layer or parameter container

@ksingla025
Copy link
Author

ksingla025 commented Sep 26, 2018

I think I am getting you. BNLSTMCell is just a declaration of a class. Still when you say call BNLSTMCell.call(), it will still call same set of parameters which are already defined in the graph at word level as they fall in same namescope of defined parameters. So treated as one unique set of parameters, not two (word and sentence).

I am seeing a big performance difference by making this change. ( i am trying in a little more complex problem of multi-class multi-label )

cell_word = BNLSTMCell(40, is_training) # h-h batchnorm LSTMCell
cell = GRUCell(30)
cell_word = MultiRNNCell([cell]*5)

cell_sent = BNLSTMCell(40, is_training) # h-h batchnorm LSTMCell
cell = GRUCell(30)
cell_sent = MultiRNNCell([cell]*5)

Similarly if you expect a different cell for forward and backward then you should define two more cells.

please correct me if I am wrong.

Other small side thing, according to my understanding, it's a general practice to not to dropout at eval time but it's happening as defined in the code here.

@ematvey
Copy link
Owner

ematvey commented Sep 26, 2018

You might be right, my memory of TF's conventions is quite vague at this point.

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

No branches or pull requests

2 participants