kan-bayashi/ParallelWaveGAN

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).

padding=(kernel_size - 1) // 2,

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.