Updated docstring for LSTM layer - Specifying order of outputs #11726
Conversation
… the state elements (hidden vs. cell state) when using return_state=True.
|
Thank you for the PR. We currently have issues with travis, causing tests to fail. It's not due to your pull request. Please be patient while we fix the broken tests. |
| @@ -2096,7 +2096,8 @@ class LSTM(RNN): | |||
| return_sequences: Boolean. Whether to return the last output | |||
| in the output sequence, or the full sequence. | |||
| return_state: Boolean. Whether to return the last state | |||
| in addition to the output. | |||
| in addition to the output. The returned elements of the | |||
There was a problem hiding this comment.
Thank you for the PR, @eyalzk. How about adding related descriptions into L293-299 (class RNN)? For example, the number of state tensors is 1 (RNN and GRU) or 2 (LSTM).
There was a problem hiding this comment.
Thanks for your reply @taehoonlee .
I agree that extra reference in the RNN layer docstring is a good idea.
However, the RNN layer can potentially be used with a custom RNN cell with more than 2 defined states. So what I would change in your suggested phrasing, is just to include the 'For example' part :)
For example, the number of state tensors is 1 (RNN and GRU) or 2 (LSTM)
What do you think?
There was a problem hiding this comment.
@eyalzk, Good point. I agree.
There was a problem hiding this comment.
Great! @taehoonlee
Added a new commit.
…-team#11726) * Updated the doc for LSTM layer. It was not clear what is the order of the state elements (hidden vs. cell state) when using return_state=True. * Updated doc for RNN layer accordingly.
Summary
When using the LSTM layer with
return_state = Truethe last states are returned in addition to the output of the layer. However, it is not clear from the documentation what is the order of the returned states (hidden state vs. cell state). For example, in:output, state_1, state_2 = LSTM(100, return_state=True)(x)I would like to know which of state_1\2 is the last hidden state and which one is the cell state.
PR Overview
Following the code I'm pretty sure I got the order right - [hidden state, cell state]. However if you decide to include this change it would be nice for another set of eyes to make sure the order is correct.