{"action":"created","issue":{"url":"https://api.github.com/repos/fchollet/keras/issues/6928","repository_url":"https://api.github.com/repos/fchollet/keras","labels_url":"https://api.github.com/repos/fchollet/keras/issues/6928/labels{/name}","comments_url":"https://api.github.com/repos/fchollet/keras/issues/6928/comments","events_url":"https://api.github.com/repos/fchollet/keras/issues/6928/events","html_url":"https://github.com/fchollet/keras/pull/6928","id":234993205,"number":6928,"title":"Yield Op support: model.fit() directly accepts TF tensors, TFRecords, and RecordInput","user":{"login":"ahundt","id":55744,"avatar_url":"https://avatars2.githubusercontent.com/u/55744?v=3","gravatar_id":"","url":"https://api.github.com/users/ahundt","html_url":"https://github.com/ahundt","followers_url":"https://api.github.com/users/ahundt/followers","following_url":"https://api.github.com/users/ahundt/following{/other_user}","gists_url":"https://api.github.com/users/ahundt/gists{/gist_id}","starred_url":"https://api.github.com/users/ahundt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ahundt/subscriptions","organizations_url":"https://api.github.com/users/ahundt/orgs","repos_url":"https://api.github.com/users/ahundt/repos","events_url":"https://api.github.com/users/ahundt/events{/privacy}","received_events_url":"https://api.github.com/users/ahundt/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":22,"created_at":"2017-06-10T10:02:35Z","updated_at":"2017-06-19T19:48:19Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/fchollet/keras/pulls/6928","html_url":"https://github.com/fchollet/keras/pull/6928","diff_url":"https://github.com/fchollet/keras/pull/6928.diff","patch_url":"https://github.com/fchollet/keras/pull/6928.patch"},"body":"**Update:** Woohoo, It all works! :-)\r\n\r\n### summary \r\n\r\nThis PR adds support for yield ops to Keras plus an example utilizing TFRecords. Correct support for yield ops in `Model` adds valuable functionality not currently supported by Keras for the reasons detailed below. \r\n\r\n#### yield ops\r\n\r\nYield ops, such as [RecordInput](https://github.com/tensorflow/tensorflow/blob/833252af72af56661aefb0541163109132f9d4a6/tensorflow/python/ops/data_flow_ops.py#L2137) ([test code](https://github.com/tensorflow/tensorflow/blob/833252af72af56661aefb0541163109132f9d4a6/tensorflow/python/kernel_tests/record_input_test.py#L70)), are different from `tf.Variable` because they provide data entirely on the C++ side when run without `fetches` or `feed_dict`, and are thus extremely efficient for large data like images. \r\n\r\n#### changes\r\n\r\nHere are the changes, marked with ==bugfix== and ==new param== in the comments below:\r\n\r\n```python\r\n# tf yield ops that supply dataset images and labels\r\nx_train_batch, y_train_batch = read_and_decode_recordinput(...)\r\n\r\n# create a basic cnn\r\nx_train_input = Input(tensor=x_train_batch)\r\nx_train_out = cnn_layers(x_train_input)\r\n\r\n# y label batch is input & output\r\n# Perhaps this aspect of API usage can be improved?\r\ny_train_in_out = Input(tensor=y_train_batch)\r\n\r\n# ==new param== + ==bugfix==\r\n# A new param `labels` has been added\r\n# so a tensor that generates labels can be\r\n# handled correctly.\r\n#\r\n# This call causes a crash without this patch because\r\n# an invalid call is made that is equivalent to:\r\n# K.placeholder(dtype=x_train_input)\r\ntrain_model = Model(inputs=[x_train_input],\r\n                    outputs=[x_train_out],\r\n                    labels=[y_train_in_out])\r\n\r\n# ==bugfix==\r\n# This call will crash without this patch because\r\n# it is assumed the parameters `x` and `y` are\r\n# provided here and not via the ops\r\n# x_train_batch and y_train_batch \r\ntrain_model.compile(optimizer='rmsprop',\r\n                    loss='categorical_crossentropy',\r\n                    metrics=['accuracy'])\r\n\r\n# ==bugfix==\r\n# This call will crash without this patch because\r\n# the changes in tensor order caused by the\r\n# constructor update, which accepts yield ops,\r\n# were not previously accounted for.\r\ntrain_model.fit(None,None)\r\n```\r\n\r\n#### TFRecord\r\n\r\nTFRecord support is a side effect of yield op support, and [examples/mnist_tfrecord.py](https://github.com/ahundt/keras/blob/b7d44a54ffee3391135abd3a3566a9c0d20a2fa8/examples/mnist_tfrecord.py) demonstrates usage.\r\n\r\n\r\n**Update:** I've moved the `fetches` and `feed_dict` public API design into #6974. This PR now focuses more narrowly on fixing bugs that occur when a tf `yield_op` is supplied as a parameter.\r\n"},"comment":{"url":"https://api.github.com/repos/fchollet/keras/issues/comments/309552857","html_url":"https://github.com/fchollet/keras/pull/6928#issuecomment-309552857","issue_url":"https://api.github.com/repos/fchollet/keras/issues/6928","id":309552857,"user":{"login":"fchollet","id":710255,"avatar_url":"https://avatars0.githubusercontent.com/u/710255?v=3","gravatar_id":"","url":"https://api.github.com/users/fchollet","html_url":"https://github.com/fchollet","followers_url":"https://api.github.com/users/fchollet/followers","following_url":"https://api.github.com/users/fchollet/following{/other_user}","gists_url":"https://api.github.com/users/fchollet/gists{/gist_id}","starred_url":"https://api.github.com/users/fchollet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fchollet/subscriptions","organizations_url":"https://api.github.com/users/fchollet/orgs","repos_url":"https://api.github.com/users/fchollet/repos","events_url":"https://api.github.com/users/fchollet/events{/privacy}","received_events_url":"https://api.github.com/users/fchollet/received_events","type":"User","site_admin":false},"created_at":"2017-06-19T19:48:19Z","updated_at":"2017-06-19T19:48:19Z","body":"Regarding this PR, I would suggest removing the `labels` argument and introducing remaining bug fixes via one or more smaller PRs."}}
{"actor":{"display_login":"fchollet"}}