Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[TTS] Add tutorials for FastPitch TTS speaker adaptation with adapters #6431
[TTS] Add tutorials for FastPitch TTS speaker adaptation with adapters #6431
Changes from 28 commits
a2c1708
9692b7d
d7567f0
76571f7
3361eab
c33c188
e0d7f1f
a57e12a
2ad34f8
49f0f02
ccc58bd
0631e5e
d3e20d9
ea85082
55bd14b
7f7fa26
6dc0ff4
94afa02
e50a603
f43a9f1
b58c677
1ae1250
0c2cbc0
e71387e
60555ab
e9ba7f6
4795fe1
87a430e
4b375d9
908aa67
f402847
4e75a6e
1cb4727
3053a52
9037e0c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have the check be if
precomputed_embedding_dim
is not None, instead of a separateprecompute
flag?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If user want to use precompute embedding, I think setting a bool value is better than setting a dimension value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having 2 parameters that are coupled but optional is awkward. If you keep it as is there should be an assert which validates that the dimension is None when the flag is false, or dimension is not None when the flag is true (similar to the XOR in https://github.com/NVIDIA/NeMo/blob/main/nemo/collections/tts/data/dataset.py#L443-L445).
Normally one would provide dependent arguments as a single module or as a config object to ensure user provides all or none of them, but in this case there is only 1 argument to configure so checking if its None is also fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use only one parameter
precomputed_embedding_dim