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

Let keras-to-tfjs conversion put all weights in a single weight group by default #193

Merged
merged 7 commits into from
Aug 5, 2018

Conversation

caisq
Copy link
Collaborator

@caisq caisq commented Aug 3, 2018

Previously, the weights from a keras model is put in separate weight groups for
separate layers oof the model, leading to a large number of weight files for a
model with many layers.

This PR changes the behavior by storing the weights from all layers of a model
in the same weight group by default. Of course, the weight group may still
be sharded into different files, if the total weight size exceeds the 4-MB shard
size limit.

The added --split_weights_by_layer flag allows users to use the legacy behavior.
E.g.,

tensorflowjs_converter --input_format keras --split_weights_by_layer \
    my_model.h5 my_tfjs_model/

Fixes: tensorflow/tfjs#570


This change is Reviewable

@caisq
Copy link
Collaborator Author

caisq commented Aug 3, 2018

Fixes: tensorflow/tfjs#570

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.

Thank you! LGTM with one tiny comment

Reviewed 3 of 5 files at r1, 2 of 2 files at r2.
Reviewable status: 0 of 1 approvals obtained (waiting on @caisq, @nsthorat, @dsmilkov, and @pyu10055)


python/tensorflowjs/converters/keras_h5_conversion.py, line 196 at r2 (raw file):

    Args:
      h5file: An instance of h5py.File, or the path to an h5py file.
      split_by_layer: TODO(cais): DO NOT SUBMIT.

this todo looks like leftover code.

Copy link
Collaborator Author

@caisq caisq left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @caisq, @nsthorat, and @pyu10055)


python/tensorflowjs/converters/keras_h5_conversion.py, line 196 at r2 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

this todo looks like leftover code.

Oops. Thanks for catching this. Fixed.

@caisq caisq merged commit 35205e3 into tensorflow:master Aug 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants