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

Wrong super call in RecurrentSequential __init__ #92

Open
JulianStj opened this issue Jan 31, 2018 · 1 comment
Open

Wrong super call in RecurrentSequential __init__ #92

JulianStj opened this issue Jan 31, 2018 · 1 comment

Comments

@JulianStj
Copy link

In the init function for RecurrentSequential, it calls

super(RecurrentModel, self).init(**kwargs)

But RecurrentSequential is inherited from RecurrentModel. The call should be

super(RecurrentSequential, self).init(**kwargs)

But the arguments don't match. I'm not quite sure how to fix this the "right" way, but calling the base class constructor is not correct for RecurrentSequential

@kklemon
Copy link

kklemon commented May 31, 2018

I also encountered this problem while trying to run the Nesting RecurrentSequentials demo. Indeed that super call in RecurrentSequential looks suspicious and should probably call RecurrentModel's constructor instead of itself, but this also wouldn't really make sense.

In opposite to Keras' Sequential model which updates itself with every add(...) call and hence doesn't need to be build in the end explicitly, RecurrentSequential doesn't and thus you need to call build(...) on the instance before you can access the internal model.

For example the mentioned demo would go down to

rnn1 = RecurrentSequential()
rnn1.add(LSTMCell(16, input_dim=30))
rnn1.add(Dense(20))
rnn1.build((None, None, 30))  # batch len, seq len, seq dim

rnn1_cell = rnn1.get_cell()

...

then.

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