-
Notifications
You must be signed in to change notification settings - Fork 2
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
Make compatible with new keras Callback model #5
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5 +/- ##
===========================================
+ Coverage 22.77% 54.54% +31.77%
===========================================
Files 3 3
Lines 101 121 +20
===========================================
+ Hits 23 66 +43
+ Misses 78 55 -23
Continue to review full report at Codecov.
|
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.
Could you indicate what versions of keras the old vs new api would be compatible to? Also, maybe update the Callback
docstring - tbh the only reason the base class is here is to avoid importing it from keras
, hence avoiding keras
as a dependency. However, if there are API differences, we could specify a keras version range in the setup.py?
6d5ac85
to
dcc4ec5
Compare
I've updated to be backwards compatible (much like tensorflow.keras does). I considered just adding the keras dependency and importing the Callback base class, but to install tf.keras it needs all of tensorflow, and it seems kinda heavyweight now that its compatible across versions. I also added a test - it requires adding matplotlib & ipython as dev dependencies though so it can import |
dcc4ec5
to
6e999b2
Compare
typeshed was recently removed from PyPI (and comes bundled with mypy anyway) so was causing dependency resolution to fail.
see https://keras.io/guides/writing_your_own_callbacks/ for guide - methods have been split to provide a different one for train/eval/predict time. This change translates `on_batch_end` to `on_train_batch_end` rather than also for validation batches, so batch history only includes train batch loss.
6e999b2
to
5d9eb03
Compare
see https://keras.io/guides/writing_your_own_callbacks/ for guide - methods have been split to provide a different one for train/eval/predict time.
This change translates
on_batch_end
toon_train_batch_end
rather than also for validation batches, so batch history only includes train batch loss - not sure if you wanna include the validation batches in the history here for plotting? Would just need to seton_test_batch_end
to the same ason_train_batch_end
.