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

Fixes wrong output dimensions in ConvTranspose1d #78

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ecobost
Copy link

@ecobost ecobost commented Jun 28, 2024

Solves #42, #58 and #68: all related to incorrect computation of output shape in the ConvTraspose1d of the DecoderBlock (as also pointed out in PR #44).

When using stride > 1 in a conv operation the output dimensions are underdetermined and ConvTranspose1d needs extra info (the output_padding) to compute the expected output (see note in docs).

Given the construction constraints of the conv/deconv operations (namely, kernel_size=stride/2, padding=ceil(stride/2)), I figured out the right output_padding (so we always recover the same input dimensions) is:

if s is even:
	output_padding = 0 if input_timesteps is divisible by stride, else 1
If stride is odd:
	output_padding = 0  if input_timesteps + 1 is divisible by stride, else 1

with input_timesteps = timestestep dimension of the input to the original conv1d.

This PR sets output_padding=0 for even strides and 1 for odd strides. This will work in the vast majority of cases (including for all pretrained models) except when:
1: if stride is even and input_timesteps is not divisible by stride.
2: if stride is odd and input_timesteps+1 is divisible by stride.
Both of which are unlikely ( and case 1 would fail anyway even without this PR). At the very least, I believe the current setting is a more sensible default.

Changes output_padding to deal better with odd strides.
plenty of changes since 1.0.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant