is_placeholder() implemented #7066

ahundt · 2017-06-21T04:41:04Z

small part of #6928 and #7046

is_placeholder()

I use is_placeholder() to differentiate numpy inputs from tensorflow op inputs, which allows y_true and y_pred to be correctly assigned to the appropriate tf op, without this change the program crashes by trying to incorrectly pass an op to K.placeholder.

is_placeholder can also be useful when enumerating the layers, inputs, outputs, and transforming an arbitrary model. For example anything that walks the graph may or may not want to modify placeholders, such as an automated labeling model to segmentation model converter.

The implementation of is_placeholder was based on _uses_learning_phase and _keras_shape.

small part of keras-team#6928 and keras-team#7046

hgaiser · 2017-07-20T09:15:26Z

Can you explain the use of is_placeholder to me? I noticed its use in examples/variational_autoencoder.py and looked up where it is used in master, looked through the changes of your PR, but its still unclear what it does. Why is it necessary in that example and when should I use it?

PavlosMelissinos · 2017-07-20T09:44:04Z

keras/backend/cntk_backend.py

@@ -2,6 +2,7 @@
 import cntk as C
 import numpy as np
 from .common import _FLOATX, _EPSILON, image_dim_ordering, image_data_format
+from .common import is_placeholder


from .common import is_placeholder

is this necessary?

Yes, I think this import is necessary, how else could it be provided in the backend? Is there a better option?

If the goal is just to be able to use K.is_placeholder, then I think it's better to add the from .common import is_placeholder to __init__.py

So you add it there once and it's available for every backend.

Imports from line 4 are used in the module, so it makes sense to have it there for each backend separately. I don't think that applies in this case.

good point, removed.

PavlosMelissinos · 2017-07-20T09:46:42Z

keras/backend/tensorflow_backend.py

@@ -14,6 +14,7 @@
 from .common import floatx
 from .common import _EPSILON
 from .common import image_data_format
+from .common import is_placeholder


from .common import is_placeholder

Again, is this necessary?

will follow conclusion from linked conversation

PavlosMelissinos · 2017-07-20T09:47:45Z

keras/backend/theano_backend.py

@@ -17,6 +17,7 @@

 import numpy as np
 from .common import _FLOATX, floatx, _EPSILON, image_data_format
+from .common import is_placeholder


will follow conclusion from linked conversation

ahundt

@hgaiser is_placeholder is used to differentiate inputs where a numpy array is expected from inputs where one is not expected because the array is provided directly from a tensor such as the tf RecordInput op. This is used internally within #6928.

As for the existing example that uses is_placeholder, I don't actually know, I just updated it with the new API. :-)

When would you use it aside from the internal need in #6928? It would make sense in graph modifying APIs or any other code that needs meta-information to instantiate or modify tensors and numpy arrays correctly such as in a heterogeneous list.

ahundt · 2017-07-20T23:36:29Z

keras/backend/cntk_backend.py

@@ -2,6 +2,7 @@
 import cntk as C
 import numpy as np
 from .common import _FLOATX, _EPSILON, image_dim_ordering, image_data_format
+from .common import is_placeholder


Yes, I think this import is necessary, how else could it be provided in the backend? Is there a better option?

ahundt · 2017-07-20T23:37:23Z

keras/backend/tensorflow_backend.py

@@ -14,6 +14,7 @@
 from .common import floatx
 from .common import _EPSILON
 from .common import image_data_format
+from .common import is_placeholder


will follow conclusion from linked conversation

ahundt · 2017-07-20T23:37:34Z

keras/backend/theano_backend.py

@@ -17,6 +17,7 @@

 import numpy as np
 from .common import _FLOATX, floatx, _EPSILON, image_data_format
+from .common import is_placeholder


will follow conclusion from linked conversation

ahundt · 2017-07-25T08:07:35Z

@PavlosMelissinos Could you confirm that I've addressed everything? Thanks for the review!

PavlosMelissinos · 2017-07-25T09:51:29Z

_is_placeholder is a Layer/Node attribute. Therefore K.is_placeholder accepts a tensor, a layer or a node.
1. I think the docstring and the parameter name should be updated to reflect this.
2. Since the tensor parameter can either be a tensor or a layer/node object, do you prefer to leave the function like this or do you think it should be a class method of Layer/Node? In the second case, the calls would become layer.is_placeholder(), so you'd get away with having to define the parameters and it would still work for any backend. Take a look at the property decorator as well.
For which layers/nodes/tensors should _is_placeholder be set to True? It is False by default, so in case the user would like to enable it (should they be allowed to do so), the 'writing your own keras layers' page should be updated with an example that takes advantage of the placeholder functionality and a brief description of its role.

ahundt · 2017-07-26T06:57:35Z

I think the docstring and the parameter name should be updated to reflect this.

How about this?

Returns if a Keras Tensor, Layer, or Node contains a `K.placeholder()`.

do you think it should be a class method of Layer/Node?

I think keeping is_placeholder() separate lowers the cognitive load for users, they don't have to know about it unless they need it and putting it in the Layer API might be overly prominent.

For which layers/nodes/tensors should _is_placeholder be set to True?

It should be True when the entity has a K.placeholder() under the hood.

'writing your own keras layers' page should be updated with an example

Good idea, I'll add this when I have a chance.

PavlosMelissinos · 2017-07-26T08:18:44Z

How about this?

Returns if a Keras Tensor, Layer, or Node contains a K.placeholder().

Yeah, something like that or as a more complete keras docstring:

"""Returns whether x contains a K.placeholder().
# Arguments
x : Tensor, Layer or Node.
# Returns
True if x is a Keras Tensor, Layer or Node and contains a K.placeholder(), False otherwise.
# Example
    >>> from keras import backend as K
    >>> a = K.placeholder((2, 2), sparse=False)
    >>> print(K.is_placeholder(a))
    True
"""

I think keeping is_placeholder() separate lowers the cognitive load for users, they don't have to know about it unless they need it and putting it in the Layer API might be overly prominent.

I'm not sure that's the case:

cognitive load seems the same to me, except for code completion maybe where there'll be one extra entry.
most users won't have to know about it since it's going to be a member method of an ancestor class (Layer/Node/Tensor).
I don't know about prominence, it's just a few extra lines.

My main objection is redundancy because it would have to be implemented three times instead of once.

I think I'm slightly in favor of turning it into a class member mostly due to encapsulation but redundancy is an issue so yeah, it's not a big deal, either solution sounds ok to me!

It should be True when the entity has a K.placeholder() under the hood.

Last (tangential and slightly off-topic) question: Would it make sense to determine this on the spot? Flags are fast (they don't require any computation) but they're more prone to bugs in general. I know the flag was there before the PR and this is probably out of scope but I'm just wondering if maybe there's a better way.

fchollet · 2017-07-31T23:36:50Z

keras/engine/topology.py

@@ -1621,7 +1635,7 @@ def __init__(self, inputs, outputs, name=None):
                                                   i,
                                                   layer.__class__.__name__))
            self.input_names.append(layer.name)
-            if layer.is_placeholder:
+            if K.is_placeholder(layer):


if getattr(layer, 'is_placeholder', False):

… getattr checks

ahundt · 2017-08-01T02:58:07Z

Based on a conversation with @fchollet there is no longer an independent K.is_placeholder() function, since it can be substituted easily with getattr(keras_placeholder, 'is_placeholder', False). Therefore, the separate function has been removed, and all areas correcting where object.is_placeholder was not set have been merged to #7067. Closing this pull request accordingly.

is_placeholder() implemented

54241d5

small part of keras-team#6928 and keras-team#7046

ahundt mentioned this pull request Jun 21, 2017

reorganize training.py, code is clearer and handles more corner cases… #7072

Closed

ahundt added 4 commits June 21, 2017 01:31

test is_placeholder()

606f30d

is_placeholder theano_backend.py

67aa1cd

extra is_placeholder() tests

722e131

Merge branch 'master' into is_placeholder

6c776c7

Dref360 mentioned this pull request Jul 2, 2017

Native TensorFlow input queue implemented with fit, predict, evaluate (TFRecords) #7208

Closed

PavlosMelissinos reviewed Jul 20, 2017

View reviewed changes

ahundt commented Jul 20, 2017

View reviewed changes

remove extraneous imports of is_placeholder

b4fb086

ahundt mentioned this pull request Jul 26, 2017

training.py _standardize_input_data() cleanup input tensor corner cases #7067

Closed

fchollet reviewed Jul 31, 2017

View reviewed changes

K.is_placeholder() becomes simple member variable is_placeholder with…

4d6e3d4

… getattr checks

ahundt force-pushed the is_placeholder branch from a0aeec3 to 4d6e3d4 Compare August 1, 2017 01:22

ahundt closed this Aug 1, 2017

turgut090 mentioned this pull request May 27, 2020

Update "k_is_placeholder" in backend.R rstudio/keras3#1047

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

is_placeholder() implemented #7066

is_placeholder() implemented #7066

ahundt commented Jun 21, 2017

hgaiser commented Jul 20, 2017

PavlosMelissinos Jul 20, 2017

ahundt Jul 20, 2017 •

edited

Loading

PavlosMelissinos Jul 21, 2017 •

edited

Loading

ahundt Jul 25, 2017 •

edited

Loading

PavlosMelissinos Jul 20, 2017

ahundt Jul 20, 2017

ahundt Jul 25, 2017

PavlosMelissinos Jul 20, 2017

ahundt Jul 20, 2017

ahundt Jul 25, 2017

ahundt left a comment

ahundt Jul 20, 2017 •

edited

Loading

ahundt Jul 20, 2017

ahundt Jul 20, 2017

ahundt commented Jul 25, 2017

PavlosMelissinos commented Jul 25, 2017 •

edited

Loading

ahundt commented Jul 26, 2017

PavlosMelissinos commented Jul 26, 2017 •

edited

Loading

fchollet Jul 31, 2017

ahundt commented Aug 1, 2017 •

edited

Loading

is_placeholder() implemented #7066

is_placeholder() implemented #7066

Conversation

ahundt commented Jun 21, 2017

hgaiser commented Jul 20, 2017

Choose a reason for hiding this comment

ahundt Jul 20, 2017 • edited Loading

Choose a reason for hiding this comment

PavlosMelissinos Jul 21, 2017 • edited Loading

Choose a reason for hiding this comment

ahundt Jul 25, 2017 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahundt left a comment

Choose a reason for hiding this comment

ahundt Jul 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahundt commented Jul 25, 2017

PavlosMelissinos commented Jul 25, 2017 • edited Loading

ahundt commented Jul 26, 2017

PavlosMelissinos commented Jul 26, 2017 • edited Loading

Choose a reason for hiding this comment

ahundt commented Aug 1, 2017 • edited Loading

ahundt Jul 20, 2017 •

edited

Loading

PavlosMelissinos Jul 21, 2017 •

edited

Loading

ahundt Jul 25, 2017 •

edited

Loading

ahundt Jul 20, 2017 •

edited

Loading

PavlosMelissinos commented Jul 25, 2017 •

edited

Loading

PavlosMelissinos commented Jul 26, 2017 •

edited

Loading

ahundt commented Aug 1, 2017 •

edited

Loading