Refactor Conv1d's `padding` with `same`
Closed this issue · 1 comments
Summary
Current HiFi-GAN implementation calculate padding
manually in Conv1d
.
We can refactor them with same
argument.
This can make the codes intuitive, and enable future big refactoring of use_causal_conv
-related redundant codes.
I made a pull request.
Current Status
Current HiFi-GAN implementation calculate padding
of Conv1d(stride=1)
as (kernel_size - 1) // 2
.
This manual calculation intend to keep signal length (time dimension size).
But, PyTorch already provide argument padding="same"
for same purpose.
padding='same' pads the input so the output has the shape as the input. (ref).
So, current implementation has redundant manual calculation.
There are 9 redundant parts.
Proposal
Just replace padding = (kernel_size - 1) // 2
with "same"
.
HiFi-GAN already guarantee 'k is odd' with assert, so there is no behavior change.
So, this is not breaking change, but just refactoring.
Merits
We can get two advange with this refactoring.
- clean code
- future big refactoring
At first, we can replace manual calculation with intuitive "same" string. Code becomes clean.
More importantly, this opens the opportunity to remove redundant if/else caused by use_causal_conv
.
HiFi-GAN has many if/else, and it is caused by padding
argument in Causal mode.
Some refactoring after padding="same"
can delete these if/else (totally 50~ line reduction).
PR
I made the pull request (#396).
Closed by un-marged.
In detail, check closed PR.