Skip to content
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

Fix stop_gradient inconsistent API #7416

Merged
merged 1 commit into from
Jul 25, 2017

Conversation

angeloskath
Copy link
Contributor

The stop_gradient documentation states that the argument should be a list of
variables. The Theano implementation crashes if the argument is a list of
variables and the CNTK implementation crashes regardless but expects a list anyway (since it passes it to combine). The TensorFlow implementation expects one variable but in case a list is passed it doesn't crash as long as the shapes are all the same.

This commit handles both cases as can be expected. The following code illustrates the current Keras behaviour.

val = np.random.rand(10, 3)
backends = [KTH, KTF, KC]
as = [k.variable(val) for k in backends]
bs = [k.square(a) for k, a in zip(backends, as)]

a, b = as[1], bs[1]
KTF.stop_gradient(a)  # works
KTF.stop_gradient([a, b])  # works but returns a tensor?

a, b = as[0], bs[0]
KTH.stop_gradient(a)  # works
KTH.stop_gradient([a, b])  # crashes

a, b = as[2], bs[2]
KC.stop_gradient(a)  # crashes
KC.stop_gradient([a, b])  # crashes

This PR makes the behaviour consistent. When passed a single tensor a single tensor is returned. When passed a list or a tuple a list is returned.

Although returning a list and expecting a list would be more consistent with the previous documentation it wouldn't be consistent with the code's behaviour which could result in breaking others' code. This change now is backwards compatible but makes the behaviour consistent and documents it.

The stop_gradient documentation states that the argument should be a list of
variables. The Theano implementation crashes if the argument is a list of
variables and the CNTK implementation crashes if it is not.

This commit handles both cases as can be expected.
@fchollet fchollet merged commit 0bc856f into keras-team:master Jul 25, 2017
@angeloskath angeloskath deleted the stop-gradient-list branch July 25, 2017 08:15
ahundt added a commit to ahundt/keras that referenced this pull request Jul 26, 2017
* commit '84ceb94055b831c486dbf4955fdf1ba0f63320d1': (42 commits)
  Fix conv reccurent test
  Style fix in conv recurrent tests.
  Support return_state parameter in ConvRecurrent2D (keras-team#7407)
  Small simplification in ResNet50 architecture
  Update FAQ with info about custom object loading.
  add example for passing in custom objects in load_model (keras-team#7420)
  Update applications.md (keras-team#7428)
  Cast constants in optimizer as floatx.
  Fix stop_gradient inconsistent API (keras-team#7416)
  Simplify static shape management in TF backend.
  Fixed warning showing up when channel axis is 1 (keras-team#7392)
  Throw exception in LSTM layer if timesteps=1 and unroll=True (keras-team#7387)
  Style fix
  Passed the scheduling argument through the `*_generator` function. (keras-team#7236)
  Fix typos. (keras-team#7374)
  Fix ImageDataGenerator.standardize to support batches (keras-team#7360)
  Fix learning phase info being left out in multi-input models (keras-team#7135)
  Fix PEP8
  Fix deserialization bug with layer sharing at heterogenous depths
  Bug fix: Support multiple outputs in Lambda layer (keras-team#7222)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants