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

[report] I think this is a bug. #11

Open
CODEJIN opened this issue Apr 16, 2021 · 2 comments
Open

[report] I think this is a bug. #11

CODEJIN opened this issue Apr 16, 2021 · 2 comments

Comments

@CODEJIN
Copy link

CODEJIN commented Apr 16, 2021

Hi, guys.

Thank you so much about sharing this code. And, I think I found a minor bug, so I am reporting it.

https://github.com/mindslab-ai/cotatron/blob/38079aa2c95d647ec915ec6e8102ae5653623b78/modules/tts_decoder.py#L64-L65

I think the prenet depth parameter must be hp.depth.prenet, not hp.depth.encoder, is it right?
Please check it.

Thanks,

Heejo

@seungwonpark
Copy link
Contributor

What a big mistake. Thanks for letting me know about this!

You're right and the depth parameter should be hp.depth.prenet here. So eventually the PreNet MLP was deeper than it originally had to be. (Intended 80->256->256 but was 80->256->256->256)

Typically, being a deeper model than intended won't affect the performance of the network much. However, in this case I think it might have changed some train-time behavior. I cannot guess whether it had positive/negative effect on the performance, so users might want to fix this error before training, or simply use the code with an error.

@wookladin Could you please pin this issue? Making a bugfix commit will break the pre-trained model, and apparently we don't have a budget for re-training the model. So we'd better make sure that the users to become aware of this bug, while acknowledging Heejo for reporting this issue.

@CODEJIN fyi I don't have a write access to this repo so I'm asking Kang-wook to do something for this repo.

@wookladin wookladin pinned this issue Apr 20, 2021
@wookladin
Copy link
Contributor

I pinned this issue. Thanks for letting us know!

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

No branches or pull requests

3 participants