Model() yield op Input internal fixes, plus add K.is_placeholder() #7046
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ahundt
added a commit
to ahundt/keras
that referenced
this pull request
Jun 21, 2017
Merge commit 'ad9283d08e5415981bea12e7d32790dc5e3b288c' into is_placeholder
ahundt
added a commit
to ahundt/keras
that referenced
this pull request
Jun 21, 2017
small part of keras-team#6928 and keras-team#7046
This was referenced Jun 21, 2017
Closed
|
only failure was flaky test #7033, closing and reopening |
ahundt
added a commit
to ahundt/keras
that referenced
this pull request
Jun 21, 2017
Dref360
approved these changes
Jun 21, 2017
| @@ -27,7 +27,8 @@ | |||
|
|
|||
| def _standardize_input_data(data, names, shapes=None, | |||
| check_batch_axis=True, | |||
| exception_prefix=''): | |||
| exception_prefix='', | |||
| tensors=None): | |||
examples/mnist_tfrecord.py
Outdated
| loss=None, | ||
| metrics=['accuracy']) | ||
|
|
||
| tensorboard = TensorBoard(write_graph=True) |
There was a problem hiding this comment.
I had to disable it because without the expected_targets param, (I'd like to rename it to that from labels) much of Keras' functionality is broken in this use case. Passing callbacks=[tensorboard] crashes. It runs perfectly in #6928, with limited functionality with this PR, and not at all on master...
| y_train_in_out = Input(tensor=y_train_batch, batch_shape=y_batch_shape, name='y_labels') | ||
| cce = categorical_crossentropy(y_train_batch, x_train_out) | ||
| train_model = Model(inputs=[x_train_input], outputs=[x_train_out]) | ||
| train_model.add_loss(cce) |
There was a problem hiding this comment.
I don't think a lot of people knew about this trick. Really nice!
There was a problem hiding this comment.
Additionally I've discovered this trick has serious performance problems, see #7075 (comment). It is 20x slower than mnist_tfrecord.py in #6928
ahundt
added a commit
to ahundt/keras
that referenced
this pull request
Jun 21, 2017
|
I've split this up into many smaller PRs, the capstone change is #7072 which should link to all the smaller ones. |
…_fixes_tfrecord # Conflicts: # keras/backend/tensorflow_backend.py # keras/backend/theano_backend.py
|
This PR has been superseded by #6928 because of #7102 (comment). For that reason I'm closing this PR. |
ahundt
added a commit
to ahundt/keras
that referenced
this pull request
Aug 1, 2017
…ut_data * is_placeholder: K.is_placeholder() becomes simple member variable is_placeholder with getattr checks remove extraneous imports of is_placeholder extra is_placeholder() tests is_placeholder theano_backend.py test is_placeholder() is_placeholder() implemented small part of keras-team#6928 and keras-team#7046
ahundt
added a commit
to ahundt/keras
that referenced
this pull request
Aug 5, 2017
* mnist_tfrecord: mnist_tfrecord.py loss defaults to None in compile() lower batch size and epochs mnist_tfrecord.py indentation fix mnist_tfrecord.py add parallelism option mnist_tfrecord.py pep8 Fix mnist_tfrecord.py runtime errors mnist_tfrecord.py added (keras-team#7061, keras-team#7072, keras-team#6928, keras-team#7046)
ahundt
added a commit
to ahundt/keras
that referenced
this pull request
Aug 5, 2017
* fit_steps_per_epoch: removed extraneous line mnist_tfrecord.py add coordinator mnist_tfrecord.py fix key missing lines mnist_tfrecord.py loss defaults to None in compile() lower batch size and epochs mnist_tfrecord.py indentation fix mnist_tfrecord.py add parallelism option mnist_tfrecord.py pep8 Fix mnist_tfrecord.py runtime errors mnist_tfrecord.py added (keras-team#7061, keras-team#7072, keras-team#6928, keras-team#7046) # Conflicts: # examples/mnist_tfrecord.py
ahundt
added a commit
to ahundt/keras
that referenced
this pull request
Aug 5, 2017
…support added * standardize_input_data: replace is_placeholder with is_keras_placeholder to resolve conflict with cntk backend (keras-team#7067) keras-team#7067 (comment) training.py _standardize_input_data() accounts for input tensors (keras-team#7067) fix _feed_inputs name _standardize_input_data() now only standardizes data that is convertible to numpy array based on what is destined for the feed_dict _standardize_input_data accepts feed inputs _standardize_input_data add missing space to exception error string K.is_placeholder() becomes simple member variable is_placeholder with getattr checks remove extraneous imports of is_placeholder extra is_placeholder() tests is_placeholder theano_backend.py test is_placeholder() is_placeholder() implemented small part of keras-team#6928 and keras-team#7046 # Conflicts: # keras/engine/topology.py # keras/engine/training.py
fchollet
pushed a commit
that referenced
this pull request
Aug 11, 2017
* generic_utils.py don't crash when dealing with batched data * Progbar() unit test * mnist_tfrecord.py added (#7061, #7072, #6928, #7046) * Fix mnist_tfrecord.py runtime errors * mnist_tfrecord.py pep8 * mnist_tfrecord.py add parallelism option * reorder inputs * mnist_tfrecord.py indentation fix * lower batch size and epochs * loss defaults to None in compile() * mnist_tfrecord.py * model.fit(steps_per_epoch) added * added _check_num_samples for cases when batch_size does not apply * fix test failures * remove inaccurate warning * improved fit(steps_per_epoch) with separate internal epoch loop in _fit_loop * fit(steps_per_epoch) initial validation support * training.py pep8 * mnist_tfrecord.py fix key missing lines * mnist_tfrecord.py add coordinator * removed extraneous line * mnist_tfrecord.py and training.py clean up based on review (#7113) * mnist_tfrecord.py extended description * training.py fix test error * mnist_tfrecord.py and training.py fixed review comments, docs, and error messages (#7113) * training.py fix unit test error for steps_per_epoch * fix docstring comments from review (#7113) * training.py improve docstrings and error case
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes include
K.is_placeholder(), plus the internal changes of #6928, excluding thelabelsparameter or TFRecord support. This includes better checks to prevent data from being fed tofeed_dictorfetchesincorrectly during model.run(), improved user input sanity checks, and simplified backend function initialization.Please also consider my alternative external API proposals in #6928 (comment), but that should be kept separate from these internal improvements.