kan-bayashi / ParallelWaveGAN

Unofficial Parallel WaveGAN (+ MelGAN & Multi-band MelGAN & HiFi-GAN & StyleMelGAN) with Pytorch
https://kan-bayashi.github.io/ParallelWaveGAN/
MIT License
1.54k stars 339 forks source link

Refactor Conv1d's `padding` with `same` #395

Closed tarepan closed 1 year ago

tarepan commented 1 year ago

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).
https://github.com/kan-bayashi/ParallelWaveGAN/blob/ffaa99fe77d3b0703e5857177fd9b2ecc18cb0bd/parallel_wavegan/models/hifigan.py#L80

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.

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

tarepan commented 1 year ago

Closed by un-marged.
In detail, check closed PR.