Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

Fix wrong assumption about the order of weights in a keras Model #178

Merged
merged 2 commits into from
Jul 20, 2018

Conversation

caisq
Copy link
Collaborator

@caisq caisq commented Jul 20, 2018

Previously a wrong assumption was made about the ordering of
weight values required for the input argument to
keras.Model.set_weights() and tf.keras.Model.set_weights().

It turns out that the correct ordering is obtained by

  • iterating over Model.layers, and
  • in an inner loop, iterate over the weights of the indivisual Layers

Previously the ordering was iterating over Model.weights, which
can differ from the above-mentioned ordering in some cases, e.g.,
in a functional model with BatchNormalization Layers as covered
by the unit tests added in this PR.

BUG

Fixes: tensorflow/tfjs#511


This change is Reviewable

Previously a wrong assumption was made about the ordering of
weight values required for the input argument to
`keras.Model.set_weights()` and `tf.keras.Model.set_weights()`.

It turns out that the correct ordering is obtained by
- iterating over Model.layers, and
- in an inner loop, iterate over the weights of the indivisual Layers

Previously the ordering was iterating over Model.weights, which
can differ from the above-mentioned ordering in some cases, e.g.,
in a functional model with BatchNormalization Layers as covered
by the unit tests added in this PR.

BUG

Fixes: tensorflow/tfjs#511
Copy link

@nsthorat nsthorat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @nsthorat, @davidsoergel, @ericdnielsen, @dsmilkov, @pyu10055, and @bileschi)

Copy link
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @davidsoergel, @ericdnielsen, @dsmilkov, @pyu10055, and @bileschi)

@caisq caisq merged commit 1ad7325 into tensorflow:master Jul 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants