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

Build data_init layer under name_scope #691

Merged
merged 4 commits into from
Nov 13, 2019

Conversation

Squadrick
Copy link
Member

The original wrapped layer and the non-trainable layer created for data-dependent initialization had a clash in their namespaces. Creating the second layer under a name scope of 'data_dep_init' fixes the issue.

Fixes #624

The original wrapped layer and the non-trainable layer created for data
dependent initialization had a clash in their namespaces. Creating the
second layer under a name scope of 'data_dep_init' fixes the issue.
seanpmorgan
seanpmorgan previously approved these changes Nov 12, 2019
Copy link
Member

@seanpmorgan seanpmorgan left a comment

Choose a reason for hiding this comment

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

Great! LGTM thanks @Squadrick . Only thing I'd wonder is if we should add a test case were we write this to disk and then delete?

@Squadrick
Copy link
Member Author

@seanpmorgan
Added the test, but it uses os module. I've added a TODO to find a better way to do this. This is particularly problematic when the test fails on model.save_model, it's creating copies of model.h5 in /tmp without being deleted. But since it's /tmp, and the save file is a few kB, I figured it's alright to merge it for own.

@WindQAQ
Copy link
Member

WindQAQ commented Nov 12, 2019

Maybe use create_tempfile in tf.test.TestCase.

@Squadrick
Copy link
Member Author

@WindQAQ Thanks for the suggestion, it's a much more elegant solution. Updated the PR.

Copy link
Member

@seanpmorgan seanpmorgan left a comment

Choose a reason for hiding this comment

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

LGTM Thanks again!

@seanpmorgan seanpmorgan merged commit 8156681 into tensorflow:master Nov 13, 2019
@Squadrick Squadrick deleted the wt-norm-h5-save branch November 13, 2019 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WeightNormalization layer can't save in h5
5 participants