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

Enforce phonemizer definition for synthesis #1441

Merged
merged 7 commits into from
Mar 25, 2022
Merged

Conversation

WeberJulian
Copy link
Contributor

  • Set the phonemizer picked at train time in the config
  • Enforce phonemizer definition for synthesis

@WeberJulian WeberJulian requested a review from erogol March 25, 2022 09:15
@erogol
Copy link
Member

erogol commented Mar 25, 2022

Can we unittest it not to miss it the next time?

@@ -34,8 +34,8 @@

# train the model for one epoch
command_train = (
f"CUDA_VISIBLE_DEVICES='{get_device_id()}' python TTS/bin/train_tts.py --config_path file://{config_path} "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you remove file:// then it is not different than a regular training test. Am I wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On my system the path was incorrectly parsed, it created a folder file: in my TTS directory.
image
I'm not sure what's the expected behavior, I just thought it was a bug.
I can reverse the commit, it was just to trigger the CI since it was bugged yesterday.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we removed fsspec saving in the trainer since it broke the checkpoints uploaded to the cloud. I think we can even remove the test completely

@WeberJulian
Copy link
Contributor Author

Can we unittest it not to miss it the next time?

You mean testing that synthesis doesn't work if phonemizer is undifined?

@erogol
Copy link
Member

erogol commented Mar 25, 2022

Can we unittest it not to miss it the next time?

You mean testing that synthesis doesn't work if phonemizer is undifined?

More like checking the config.json if characters are passed as needed.

@WeberJulian WeberJulian requested a review from erogol March 25, 2022 18:31
@erogol erogol merged commit c66a624 into dev Mar 25, 2022
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.

None yet

2 participants