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

Tidied; simplified; generalised ConvTranspose implementation. #40

Merged
merged 2 commits into from
Mar 10, 2022

Conversation

patrick-kidger
Copy link
Owner

@andyehrenberg I think I've got transposed convolutions sorted out.

WDYT? If you're happy with this then I'll merge this in to the other ongoing feature branch.

The main references I used to implement this were Relation 14 of this reference and these animations.

@andyehrenberg
Copy link
Contributor

Definitely happy with this! I really like the clear explanation of output_padding. Do you think it’s worthwhile to include output_shape as an optional argument (that would have to agree with output_padding, if given), as haiku has done? While this could make model building more intuitive, specifying output_padding enables input images of varying sizes in the case of encoder-decoder style architectures, while output_shape could place too much of a restriction. I don’t think it’d be super important/useful to include it. Flax doesn’t seem to include either of these, as noted in google/flax#1872.

@patrick-kidger
Copy link
Owner Author

Excellent, glad you like it!

I agree with you that output_shape doesn't sound super important/useful. I think it's nicer to have only one way to do things.

I'll merge this PR in.

@patrick-kidger patrick-kidger merged commit f3b8b27 into attn-convt-layernorm Mar 10, 2022
@patrick-kidger patrick-kidger deleted the tweak-convtranspose branch March 10, 2022 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants