support run_options and run_metadata for tf.keras.function #19932

facaiy · 2018-06-12T08:15:09Z

Fix #19911.
cc @fchollet

caisq · 2018-06-12T13:16:00Z

tensorflow/python/keras/backend_test.py

      f = keras.backend.function(inputs=[x_placeholder, y_placeholder],
                                 outputs=[x_placeholder + y_placeholder],
                                 updates=[(x, x_placeholder + 1.)],
-                                 fetches=[keras.backend.update(y, 5.)])
+                                 fetches=[keras.backend.update(y, 5.)],
+                                 options=run_options)


I'd argue that this test is inadequate because it doesn't verify the effect of sending the run_options.
I suggest you use both the options and run_metadata keywords here and set output_partition_graphs of RunOptions to True. Then you can verify that the partition_graphs of run_metadata is set after the call.

I agree that it's better to add run_metadata keyword, however I'm afraid _make_callable_from_options doesn't support run_metadata yet.

callable_fn = session._make_callable_from_options(callable_opts)

https://github.com/facaiy/tensorflow/blob/a5840964e0eb422fbe73dd3738c8d14c1147276f/tensorflow/python/keras/backend.py#L2853

The solution I can see is

to replace session._make_callable_from_options by session.make_callable,

or, to add run_metadata support for TF_SessionMakeCallable.

how do you think it? cc @fchollet @mrry

@facaiy run_metadata support for TF_SessionMakeCallable has been added by @mrry 574a851

Thank you for letting me know. I'd like to add the argument in the weekend.

caisq · 2018-06-12T13:16:37Z

tensorflow/python/keras/backend.py

@@ -2759,7 +2759,8 @@ class Function(object):
      outputs: Output tensors to fetch.
      updates: Additional update ops to be run at function call.
      name: A name to help users identify what this function does.
-      session_kwargs: Arguments to `tf.Session.run()`: `fetches`, `feed_dict`.
+      session_kwargs: Arguments to `tf.Session.run()`:
+                      `fetches`, `feed_dict`, `options`.


There is run_metadata as well, which is useful for things like getting the partition graphs and step_stats.

caisq · 2018-06-12T13:16:57Z

tensorflow/python/keras/backend.py

@@ -2844,6 +2846,9 @@ def _make_callable(self, feed_arrays, feed_symbols, symbol_vals, session):
      callable_opts.fetch.append(x.name)
    # Handle updates.
    callable_opts.target.append(self.updates_op.name)
+    # Handle run_options.
+    if self.run_options:


As mentioned above, handle run_metadata as well.

fchollet · 2018-06-20T21:02:39Z

@mrry could you please take a look? Thank you!

mrry · 2018-06-20T22:06:54Z

Looks fine to me, but none of this is in code I own, so is there anything specific you'd like me to review?

…onfigs_for_keras_model

This reverts commit ffc5c4e.

facaiy · 2018-06-22T07:10:51Z

@caisq I have added run_metadata argument, thanks for the work of @mrry . Could you take a look again?

caisq · 2018-06-22T13:16:29Z

tensorflow/python/keras/backend.py

-    fetched = self._callable_fn(*array_vals)
+    if self.run_metadata:
+      fetched = self._callable_fn(*array_vals,
+                                  run_metadata=self.run_metadata)


Is it possible to always use lines 2908-2099? I'm guessing (and not sure) that if self.run_metadata is None, it'll be handled correctly?

Thank you, I think so.

tensorflowbutler · 2018-07-07T18:37:29Z

Nagging Assignee @caisq: It has been 15 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

…onfigs_for_keras_model

facaiy · 2018-07-11T04:33:07Z

@fchollet @caisq Hi, could you take a look? Thanks.

tensorflow/tensorflow#19932

tensorflow/tensorflow#19932 metadata can only be passed to the callable when the session is instrumented

tensorflowbutler · 2018-07-25T19:10:51Z

Nagging Assignee @caisq: It has been 14 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

facaiy · 2018-07-29T14:25:48Z

Hi, could anyone take a look? I think the PR has been pending for a long time. Thanks.

ahundt · 2018-08-07T04:53:09Z

This would definitely be useful to have.

facaiy · 2018-08-09T12:34:49Z

@martinwicke Could you take a look? I think the PR is pending for a long time.

pavithrasv

The code looks good to me. I am not very familiar with run_options so I will wait for one more person to review.

facaiy · 2018-08-10T05:31:44Z

@pavithrasv Thank you. Could you ping a reviewer who you think is available now?

caisq · 2018-08-10T20:22:53Z

tensorflow/python/keras/backend.py

@@ -2762,7 +2762,8 @@ class Function(object):
      outputs: Output tensors to fetch.
      updates: Additional update ops to be run at function call.
      name: A name to help users identify what this function does.
-      session_kwargs: Arguments to `tf.Session.run()`: `fetches`, `feed_dict`.
+      session_kwargs: Arguments to `tf.Session.run()`:


Sorry for the delay in getting back. This PR LGTM except for a minor comment.

session_kwarg is not precise. It should be called session_run_kwarg, otherwise it may be confused with kwargs that you can use when creating a session (e.g., https://www.tensorflow.org/api_docs/python/tf/Session)

Never mind. This is an existing name and changing it would be breaking.

facaiy · 2018-08-10T23:37:32Z

many thanks. Py2 Build failed. Could you restart it?

fchollet

Looks good to me, thank you! Sorry for the delayed review.

tensorflow/tensorflow#19932 metadata can only be passed to the callable when the session is instrumented

…as_model PiperOrigin-RevId: 209546715

facaiy added 2 commits June 12, 2018 15:52

TST: add test case

ffc5c4e

ENH: support run options for function

a584096

googlebot added the cla: yes label Jun 12, 2018

facaiy mentioned this pull request Jun 12, 2018

Adding report_tensor_allocations_upon_oom to RunOptions in Keras Ask Question #19911

Closed

caisq requested a review from fchollet June 12, 2018 13:07

caisq requested changes Jun 12, 2018

View reviewed changes

caisq added the stat:awaiting response Status - Awaiting response from author label Jun 12, 2018

caisq self-assigned this Jun 12, 2018

fchollet requested a review from mrry June 20, 2018 21:02

facaiy added 3 commits June 22, 2018 14:16

Merge remote-tracking branch 'upstream/master' into ENH/support_run_c…

13241bb

…onfigs_for_keras_model

Revert "TST: add test case"

4bb5c26

This reverts commit ffc5c4e.

ENH: add run_metadata argument

c551451

facaiy changed the title ~~support RunOptions for tf.keras.function~~ support run_options and run_metadata for tf.keras.function Jun 22, 2018

caisq reviewed Jun 22, 2018

View reviewed changes

CLN: remove redundant if...else

e12a33c

tensorflowbutler removed the stat:awaiting response Status - Awaiting response from author label Jun 23, 2018

Merge remote-tracking branch 'upstream/master' into ENH/support_run_c…

144ffee

…onfigs_for_keras_model

hermansje mentioned this pull request Jul 12, 2018

TensorBoard: reuse test function for histogram keras-team/keras#10513

Closed

hermansje added a commit to hermansje/keras that referenced this pull request Jul 19, 2018

Pass options and metadata to callable in Tensorflow backend

6375906

tensorflow/tensorflow#19932

hermansje added a commit to hermansje/keras that referenced this pull request Jul 19, 2018

Pass options and metadata to callable in Tensorflow backend

90edc2d

tensorflow/tensorflow#19932 metadata can only be passed to the callable when the session is instrumented

hermansje added a commit to hermansje/keras that referenced this pull request Jul 20, 2018

Pass options and metadata to callable in Tensorflow backend

97ad315

tensorflow/tensorflow#19932 metadata can only be passed to the callable when the session is instrumented

hermansje added a commit to hermansje/keras that referenced this pull request Jul 24, 2018

Pass options and metadata to callable in Tensorflow backend

86dbda7

tensorflow/tensorflow#19932 metadata can only be passed to the callable when the session is instrumented

jonathanking mentioned this pull request Aug 6, 2018

Chrome timeline for Keras? #9868

Closed

pavithrasv reviewed Aug 9, 2018

View reviewed changes

caisq reviewed Aug 10, 2018

View reviewed changes

caisq added the stat:awaiting response Status - Awaiting response from author label Aug 10, 2018

caisq approved these changes Aug 10, 2018

View reviewed changes

caisq added awaiting testing (then merge) ready to pull PR ready for merge process and removed stat:awaiting response Status - Awaiting response from author labels Aug 10, 2018

kokoro-team removed the kokoro:run label Aug 10, 2018

caisq added ready to pull PR ready for merge process kokoro:force-run Tests on submitted change and removed ready to pull PR ready for merge process awaiting testing (then merge) labels Aug 11, 2018

kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 11, 2018

caisq added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Aug 11, 2018

fchollet approved these changes Aug 14, 2018

View reviewed changes

hermansje added a commit to hermansje/keras that referenced this pull request Aug 20, 2018

Pass options and metadata to callable in Tensorflow backend

ca98e2d

tensorflow/tensorflow#19932 metadata can only be passed to the callable when the session is instrumented

tensorflow-copybara merged commit 144ffee into tensorflow:master Aug 21, 2018

tensorflow-copybara pushed a commit that referenced this pull request Aug 21, 2018

Merge pull request #19932 from facaiy:ENH/support_run_configs_for_ker…

eaef34c

…as_model PiperOrigin-RevId: 209546715

facaiy deleted the ENH/support_run_configs_for_keras_model branch August 21, 2018 07:19

skeydan mentioned this pull request Sep 6, 2018

tf$profile regex error rstudio/tensorflow#265

Closed

yanboliang mentioned this pull request Sep 8, 2018

Fix TF Function accepts session_kwargs: options and run_metadata keras-team/keras#11107

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support run_options and run_metadata for tf.keras.function #19932

support run_options and run_metadata for tf.keras.function #19932

facaiy commented Jun 12, 2018 •

edited

caisq Jun 12, 2018

facaiy Jun 13, 2018 •

edited

pavithrasv Jun 20, 2018

facaiy Jun 21, 2018

caisq Jun 12, 2018

caisq Jun 12, 2018

fchollet commented Jun 20, 2018

mrry commented Jun 20, 2018

facaiy commented Jun 22, 2018

caisq Jun 22, 2018

facaiy Jun 22, 2018

tensorflowbutler commented Jul 7, 2018

facaiy commented Jul 11, 2018

tensorflowbutler commented Jul 25, 2018

facaiy commented Jul 29, 2018

ahundt commented Aug 7, 2018

facaiy commented Aug 9, 2018

pavithrasv left a comment

facaiy commented Aug 10, 2018

caisq Aug 10, 2018

caisq Aug 10, 2018

facaiy commented Aug 10, 2018

fchollet left a comment

support run_options and run_metadata for tf.keras.function #19932

support run_options and run_metadata for tf.keras.function #19932

Conversation

facaiy commented Jun 12, 2018 • edited

Choose a reason for hiding this comment

facaiy Jun 13, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fchollet commented Jun 20, 2018

mrry commented Jun 20, 2018

facaiy commented Jun 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tensorflowbutler commented Jul 7, 2018

facaiy commented Jul 11, 2018

tensorflowbutler commented Jul 25, 2018

facaiy commented Jul 29, 2018

ahundt commented Aug 7, 2018

facaiy commented Aug 9, 2018

pavithrasv left a comment

Choose a reason for hiding this comment

facaiy commented Aug 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facaiy commented Aug 10, 2018

fchollet left a comment

Choose a reason for hiding this comment

facaiy commented Jun 12, 2018 •

edited

facaiy Jun 13, 2018 •

edited