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.

Sign up for GitHub

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

Jump to bottom

Transposed convolution (deconvolution) #36

Merged
merged 9 commits into from
Oct 3, 2020
Merged

Transposed convolution (deconvolution) #36

merged 9 commits into from
Oct 3, 2020

Conversation

vloncar
Copy link
Contributor

@vloncar vloncar commented Jul 22, 2020

This PR adds QConv2DTranspose which is useful in autoencoders. I can also add QConv1DTranspose but the equivalent Keras layer is only avaiable in nightly TF releases, not in stable channel yet.

@googlebot googlebot added the cla: yes contribution license agreement signed label Jul 22, 2020
@zhuangh zhuangh self-requested a review August 5, 2020 17:33
zhuangh
zhuangh suggested changes Aug 5, 2020
View reviewed changes
Copy link
Contributor

@zhuangh zhuangh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! @vloncar

Would you mind adding the corresponding functions in utils.py, for example, quantized and save the weights, etc.

examples/example_mnist_ae.py Outdated
x_train = x_train[..., np.newaxis]
x_test = x_test[..., np.newaxis]

x_train /= 255.0
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we use 256 since it is powers-of-two number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is based on the other mnist examples and they all use 255. It should be the same, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

My thought: 256 is power of two and more hardware friendly for the implementation.

@zhuangh zhuangh added the ready to pull to trigger copybara-service label Aug 5, 2020
@zhuangh zhuangh removed the ready to pull to trigger copybara-service label Aug 14, 2020
@zhuangh zhuangh self-requested a review August 14, 2020 23:13
@vloncar
Copy link
Contributor Author

vloncar commented Aug 14, 2020

@zhuangh I'll make the changes you requested over the weekend. I didn't forget, I was just working on something else.

@zhuangh
Copy link
Contributor

zhuangh commented Aug 14, 2020

@vloncar awesome. thanks!

@vloncar
Copy link
Contributor Author

vloncar commented Aug 31, 2020

@zhuangh do you have time to review this? thanks!

@zhuangh zhuangh added the ready to pull to trigger copybara-service label Sep 5, 2020
@zhuangh
Copy link
Contributor

zhuangh commented Sep 13, 2020

Thanks @vloncar . And pull into the internal system for other round of review.

@zhuangh
Copy link
Contributor

zhuangh commented Sep 29, 2020

ping @lishanok

@zhuangh zhuangh added ready to pull to trigger copybara-service and removed ready to pull to trigger copybara-service labels Sep 30, 2020
@zhuangh zhuangh self-requested a review September 30, 2020 06:12
@zhuangh
Copy link
Contributor

zhuangh commented Sep 30, 2020

Hi @vloncar could you do me a favor to resolve the conflicting file qkeras/utils.py. we want to merge this PR soon. sorry for the delay.

cc @lishanok

lishanok
lishanok reviewed Sep 30, 2020
View reviewed changes
qkeras/utils.py Outdated Show resolved Hide resolved
@zhuangh zhuangh added ready to pull to trigger copybara-service and removed ready to pull to trigger copybara-service labels Sep 30, 2020
@lishanok
Copy link
Collaborator

lishanok commented Oct 1, 2020

Hi @vloncar could you also sync qkeras/autoqkeras_internal.py with the latest code? We just did some changes to it. Thanks!

@vloncar
Copy link
Contributor Author

vloncar commented Oct 2, 2020

@lishanok I don't see any conflicts with the latest change (Activation and limits).

@zhuangh zhuangh added ready to pull to trigger copybara-service and removed ready to pull to trigger copybara-service labels Oct 3, 2020
@zhuangh
Copy link
Contributor

zhuangh commented Oct 3, 2020

I modified using e3d6778 to sync with e3d6778

@zhuangh zhuangh added ready to pull to trigger copybara-service and removed ready to pull to trigger copybara-service labels Oct 3, 2020
@zhuangh
Update utils.py
3958f7a 
fixed line-too-long
@zhuangh zhuangh added ready to pull to trigger copybara-service and removed ready to pull to trigger copybara-service labels Oct 3, 2020
@qkeras-robot qkeras-robot merged commit ebc0964 into google:master Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lishanok lishanok lishanok left review comments

@zhuangh zhuangh Awaiting requested review from zhuangh

Assignees

@vloncar vloncar

Labels
cla: yes contribution license agreement signed ready to pull to trigger copybara-service
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vloncar @zhuangh @lishanok @googlebot @qkeras-robot