Adding a new notebook for text generation using RNN #158
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
BenoitDherin
assigned sanjanalreddy and BenoitDherin and unassigned sanjanalreddy and BenoitDherin
| @@ -0,0 +1,1433 @@ | |||
| { | |||
There was a problem hiding this comment.
- " Create training examples and targets" -> " Create training examples and targets for text generation"
Reply via ReviewNB
| @@ -0,0 +1,1433 @@ | |||
| { | |||
There was a problem hiding this comment.
nit: Consider putting the comments in fully fledge sentences in markdown above rather than in the code. I personally find that too many comments within the code tend to make it harder to read, especially when it's obvious. For instance, since it's very clear that print(f"Length of text: {len(text)} characters") does, I would not add a comment explaining we are printing the length of text.
Reply via ReviewNB
There was a problem hiding this comment.
Sounds good! Cleaned up comments throughout the whole lab. Thank you!
| @@ -0,0 +1,1433 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1433 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1433 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1433 @@ | |||
| { | |||
There was a problem hiding this comment.
- I'd remove all the TODOs and reserve them for the exercise notebook.
- Maybe add a brief sentence above this class in a markdown cell, explaining that 1) we need to derive from tf.keras.Model, 2) in the constructor we define the layers, and 3) in the call method we define the pass forward using the layers defined in the constructor
- I'd remove line 14: x = inputs, and pass directly inputs to self.embedding
- Maybe add a comment on over line 16, explaining what that state logic in 16-18 does (here I think it's warranted to add the comment in the code directly). Maybe this can also be explained in a markdown cell.
Reply via ReviewNB
There was a problem hiding this comment.
Done! Took all your suggestions!
| @@ -0,0 +1,1433 @@ | |||
| { | |||
There was a problem hiding this comment.
|
Super nice notebook! It all ran fine for me. I added minor comments and suggestions. |
Adding a new notebook as part of the NLP offering. This is a really nice notebook which shows how text can be generated using an RNN. This will be a perfect stepping stone for students to learn how text generation works. This notebook was adapted from this tensorflow tutorial .
This notebook uses sub-classing, as part of the slides, I will also add a brief lecture on how subclassing is done.
Currently, only the solution notebook is part of this PR. Once I incorporate review comments, I will create a corresponding lab notebook.
Please review!