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

Fix VITS upsampling asserts #1550

Merged
merged 2 commits into from
May 12, 2022
Merged

Fix VITS upsampling asserts #1550

merged 2 commits into from
May 12, 2022

Conversation

Edresson
Copy link
Contributor

@Edresson Edresson commented May 2, 2022

Fix upsampling VITS asserts that was broken.

@Edresson Edresson changed the base branch from dev to fix-gan May 2, 2022 18:42
@Edresson Edresson changed the base branch from fix-gan to fix_mas May 2, 2022 23:39
@Edresson Edresson changed the base branch from fix_mas to dev May 2, 2022 23:40
@Edresson Edresson changed the base branch from dev to fix-gan May 2, 2022 23:57
@erogol
Copy link
Member

erogol commented May 3, 2022

I don't even see an assert fix in this PR? Do I miss something ?

@Edresson
Copy link
Contributor Author

Edresson commented May 3, 2022

I don't even see an assert fix in this PR? Do I miss something ?

It fixes the upsampling asserts during the training. Without this, we can't train the model, because depending on the length of the audio the spectrogram length multiplied by the upsampling factor is not equal to the mel-spectrogram length. So I removed some frames of the audio like @WeberJulian did for the bandwidth extension model

We have two alternatives to the training works:

  1. Apply the changes in this PR (the better in my opinion)
  2. We can remove the upsampling asserts

@erogol
Copy link
Member

erogol commented May 3, 2022

I don't see an assert removed in the changes.

@Edresson
Copy link
Contributor Author

Edresson commented May 3, 2022

I don't see an assert removed in the changes.

Because I have chosen to apply the changes in this PR. This PR fixes the asserts errors during the training keeping the asserts.

Base automatically changed from fix-gan to dev May 5, 2022 00:55
@Edresson Edresson requested a review from erogol May 6, 2022 18:45
@erogol
Copy link
Member

erogol commented May 7, 2022

I don't see an assert removed in the changes.

Because I have chosen to apply the changes in this PR. This PR fixes the asserts errors during the training keeping the asserts.

I don't understand, sorry. Where are the asserts? What assert did you change? Do you mean assert python statement?

@Edresson
Copy link
Contributor Author

Edresson commented May 7, 2022

I don't see an assert removed in the changes.

Because I have chosen to apply the changes in this PR. This PR fixes the asserts errors during the training keeping the asserts.

I don't understand, sorry. Where are the asserts? What assert did you change? Do you mean assert python statement?

These asserts 1 and 2 in some cases (depending on the audio length) break the upsampling training. So to fix this bug I submitted this PR.

@erogol
Copy link
Member

erogol commented May 7, 2022

I don't see an assert removed in the changes.

Because I have chosen to apply the changes in this PR. This PR fixes the asserts errors during the training keeping the asserts.

I don't understand, sorry. Where are the asserts? What assert did you change? Do you mean assert python statement?

These asserts 1 and 2 in some cases (depending on the audio length) break the upsampling training. So to fix this bug I submitted this PR.

Can you link to your exact changes in the code which fix those asserts? I don't understand how you fix those lines without touching those lines

@Edresson Edresson force-pushed the fix-upsampling-asserts branch 2 times, most recently from a66f13b to b51e54c Compare May 7, 2022 17:13
@Edresson
Copy link
Contributor Author

Edresson commented May 7, 2022

I don't see an assert removed in the changes.

Because I have chosen to apply the changes in this PR. This PR fixes the asserts errors during the training keeping the asserts.

I don't understand, sorry. Where are the asserts? What assert did you change? Do you mean assert python statement?

These asserts 1 and 2 in some cases (depending on the audio length) break the upsampling training. So to fix this bug I submitted this PR.

Can you link to your exact changes in the code which fix those asserts? I don't understand how you fix those lines without touching those lines

My bad I accidentally included the reinit encoder/duration predictor. Now I kept on this PR just the code that solves the asserts and added the reinit encoder/duration predictor in the PR #1562.

I don't understand how you fix those lines without touching those lines

These asserts are about the audio/spectrogram length, so if we guarantee that the audio/spectrogram is divisible by the upsampling factor, the problem is solved.

* Add reinit encoder and duration predictor option

* Add .data to prevent any overlooked autograd hook
@erogol erogol merged commit e45ae57 into dev May 12, 2022
@erogol erogol deleted the fix-upsampling-asserts branch May 12, 2022 12:51
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