-
Notifications
You must be signed in to change notification settings - Fork 752
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
Migrate to Tensorflow Keras to support TF2.1 #165
Conversation
…del, missing the ellipsis in the list of punctuations
Fixes generate_to_file using unicode characters by adding encoding="utf-8" in the open command.
Fixes generate_to_file using unicode characters
…uation, added double quotes to targeted punctuation
We really want equality here. $ __python__ ```python >>> hi = 'h' >>> hi += 'i' >>> hi 'hi' >>> hi == 'hi' True >>> hi is 'hi' False >>> 0 == 0.0 True >>> 0 is 0.0 False ```
allow_growth option for GeForce RTX could not create cuDNN issue
Identity is not the same thing as equality in Python
enabled removal of spaces around punctuation
unicode characters using python 3
Spaces for word-level training should be applied regardless of new_mo…
Did not work for GPU and Multi-GPU yet. Hold on, will try to migrate to Tensorflow distribute.MirroredStrategy() |
Alright so now it works for CPU and GPU, but when using Multi-GPU there is an error when generating samples after training.
@minimaxir Any clues to what causes this? |
This looks great! Thanks! :) [I apologize for not being productive with my approach.] I am OK with depreciating multi-GPU if necessary because I don't think that actually worked in the first place; I added it initially just for experimentation, Have you verified that models created with earlier versions import correctly? |
Thanks yourself for writing this awesome application :) No I have not verified that, I have basically just tried textgen.generate() and textgen.train_from_large_textfile() on both GPU and CPU-only. |
Did a test now.
Worked without errors :) |
(sorry for lack of response, will look at this PR this week!) |
No problem :) Just merged in the master to have the last commits also migrated to TF2.1 and switched to the Adam optimizer as it should perform better than RMSprop in all scenarios. |
Tested and LGTM, so will merge into the branch (and make a few changes before merging to master) Two issues I see:
|
This will use native Tensorflow Keras and supports Tensorflow 2.1