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

Incorrext init for LSTM #17

Closed
david-waterworth opened this issue Feb 16, 2020 · 3 comments
Closed

Incorrext init for LSTM #17

david-waterworth opened this issue Feb 16, 2020 · 3 comments

Comments

@david-waterworth
Copy link

The code below appears incorrect - I may be misunderstanding though. As per the comment you're summing 2 dense layers so one has no bias - but dense_h has bias=False and you're passing the bias_init, dense_i has bias=True but is using the default bias_init? Should be the other way around or pass bias_init to both?

Since the default bias_init is zeros for both Dense.apply() and LSTMCell.apply() it has no impact unless a different bias_init is supplied?

https://github.com/google-research/flax/blob/e7247d58e4f3460c03da5f935cb83d9c0883a97c/flax/nn/recurrent.py#L97-L103

@jheek
Copy link
Member

jheek commented Feb 17, 2020

You are right bias_init should be passed to dense_i instead. Do you want to make a PR?

@david-waterworth
Copy link
Author

david-waterworth commented Feb 17, 2020

I submitted PR #18

@avital
Copy link
Contributor

avital commented Mar 6, 2020

#18 was merged, so I'll close this. Thanks!

@avital avital closed this as completed Mar 6, 2020
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

3 participants