Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

Compression support for tf.keras.Sequential #2887

Merged
merged 3 commits into from
Sep 15, 2020

Conversation

liuzhe-lz
Copy link
Contributor

@liuzhe-lz liuzhe-lz commented Sep 11, 2020

"A little bit breaking" change: Compressor.compress() no longer guarantees to return the original model.

This is because tf.keras.Sequential is designed for read-only usage and therefore is too tricky to compress in place. So we create a new Sequential with instrumented layers instead.

@liuzhe-lz liuzhe-lz requested review from QuanluZhang and chicm-ms and removed request for QuanluZhang September 11, 2020 05:34
@scarlett2018 scarlett2018 mentioned this pull request Sep 11, 2020
79 tasks
@liuzhe-lz liuzhe-lz closed this Sep 13, 2020
@liuzhe-lz liuzhe-lz reopened this Sep 13, 2020
ret.update(_locate_layers(value, cur_path + [key]))
elif isinstance(value, tf.keras.layers.Layer):
ret[id(value)] = LayerInfo(value, cur_path + [key])
def _instrument(layer, config_list, LayerWrapperClass, compressor):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to move _instrument and other functions into Compressor class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, of course. I just personally don't like heavy methods which are used in (and can only be used in) constructor. Because I think "partially constructed" objects can be confusing.

@liuzhe-lz liuzhe-lz merged commit 10d7ece into microsoft:master Sep 15, 2020
@liuzhe-lz liuzhe-lz deleted the compress-seq branch September 15, 2020 17:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants