-
Notifications
You must be signed in to change notification settings - Fork 617
Rnn testable doctests #2147
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
Rnn testable doctests #2147
Conversation
|
You are owner of some files modified in this pull request. |
| >>> input = tf.keras.Input((100, 100)) | ||
| >>> output = tf.keras.layers.Layer(input) | ||
| <... Note: Values used for timesteps and input_dim have been used purely |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this warning? I think we can at least print output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll include the output of output but it's only going to be displayed as a Layer object. I think the example tries to depict the context in which the PeepholeLSTMCells can be utilized, it doesn't necessarily demonstrate the full process. That's why I included a short note stating that the values used for input have no significance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. Can we change the functional API to
>>> output, state = rnn(input)
>>> output
>>> stateThis is much more informative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WindQAQ Sorry for the late reply I somehow missed this comment. When I try
>>> keras.layers.RNN(peephole_lstm_cells,return_state=True)
>>> layer.statesit displays ([None, None], [None, None]). I'm a little confused on how to view the state of the layer.
|
@Harsh188 We should canonicalize the example. See the following colab for more details. Note that different cell may have different lengths of state, so when https://colab.research.google.com/drive/1Ndgy5iseSrcBm6tjQGdnLyMhbszvdAM8?usp=sharing |
|
@WindQAQ Thank you for providing the colab example! I'll get to work on this and implement them for the other rnn cells as well. |
|
Friendly ping @WindQAQ :) |
WindQAQ
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
* added doctests layer_norm_simple * added doctests peephole_lstm_cell.py * minor changes * including output for example * Updated examples to be descriptive and standardized
Description
Adding testable docstrings to
rnn.Part of #2066
Changes made to:
esn_cell.pylayer_norm_simple_rnn_cell.pypeephole_lstm_cell.pyType of change
Checklist:
How Has This Been Tested?
Using pytest