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

Add New Transformer Backbone for TTS Models #11911

Merged
merged 9 commits into from
Jan 22, 2025
Merged

Conversation

blisc
Copy link
Collaborator

@blisc blisc commented Jan 21, 2025

What does this PR do ?

Add new transformer backbone for TTS models.

  • add core functions and classes of Transformer blocks.
  • completed tests for all classes, and made bugfixes. Now all tests are passed after running,
    pytest -s -vvv tests/collections/tts/modules/test_transformer_2501.py

Collection: tts

Changelog

  • Add new transformer backbone for TTS models

PR Type:

  • New Feature

@blisc blisc added the Run CICD label Jan 21, 2025
padding=padding,
dilation=dilation,
bias=bias,
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding this initialization helps stabilize training (after this line) as per my observations torch.nn.init.normal_(self.conv.weight, mean=0.0, std=0.02)
This was there is experimentalt5tts

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So, we will not add this here. o_net is handled in Transformer init, but proj was not. I will add this init to the projection layer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

Comment on lines +663 to +664
cond: Optional[Union[torch.Tensor, List[torch.Tensor]]] = None,
cond_mask: Optional[Union[torch.Tensor, List[torch.Tensor]]] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: we now support the types of cond either torch.Tensor or a list of torch.Tensor. Is it better to unify the two as only a list of torch.Tensor? in this case, a list of a single tensor corresponds to a single encoder, and a list of multipe tensors corresponds to multiple encoders?

Copy link
Collaborator

@XuesongYang XuesongYang Jan 22, 2025

Choose a reason for hiding this comment

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

@paarthneekhara what are your thoughts on that?

blisc and others added 3 commits January 21, 2025 13:28
XuesongYang and others added 2 commits January 22, 2025 01:53
… passed after running,

`pytest -s -vvv tests/collections/tts/modules/test_transformer_2501.py`

Signed-off-by: Xuesong Yang <[email protected]>
Copy link
Contributor

beep boop 🤖: 🚨 The following files must be fixed before merge!


Your code was analyzed with PyLint. The following annotations have been identified:

************* Module nemo.collections.tts.modules.transformer_2501
nemo/collections/tts/modules/transformer_2501.py:686:0: C0301: Line too long (123/119) (line-too-long)
nemo/collections/tts/modules/transformer_2501.py:26:0: C0115: Missing class docstring (missing-class-docstring)
nemo/collections/tts/modules/transformer_2501.py:85:4: C0116: Missing function or method docstring (missing-function-docstring)
nemo/collections/tts/modules/transformer_2501.py:94:0: C0115: Missing class docstring (missing-class-docstring)
nemo/collections/tts/modules/transformer_2501.py:138:0: C0115: Missing class docstring (missing-class-docstring)
nemo/collections/tts/modules/transformer_2501.py:171:4: C0116: Missing function or method docstring (missing-function-docstring)
nemo/collections/tts/modules/transformer_2501.py:191:4: C0116: Missing function or method docstring (missing-function-docstring)
nemo/collections/tts/modules/transformer_2501.py:195:4: C0116: Missing function or method docstring (missing-function-docstring)
nemo/collections/tts/modules/transformer_2501.py:280:0: C0115: Missing class docstring (missing-class-docstring)
nemo/collections/tts/modules/transformer_2501.py:340:0: C0115: Missing class docstring (missing-class-docstring)
nemo/collections/tts/modules/transformer_2501.py:398:0: C0115: Missing class docstring (missing-class-docstring)
nemo/collections/tts/modules/transformer_2501.py:471:4: C0116: Missing function or method docstring (missing-function-docstring)
nemo/collections/tts/modules/transformer_2501.py:536:0: C0115: Missing class docstring (missing-class-docstring)
nemo/collections/tts/modules/transformer_2501.py:627:4: C0116: Missing function or method docstring (missing-function-docstring)

-----------------------------------
Your code has been rated at 9.43/10

Mitigation guide:

  • Add sensible and useful docstrings to functions and methods
  • For trivial methods like getter/setters, consider adding # pylint: disable=C0116 inside the function itself
  • To disable multiple functions/methods at once, put a # pylint: disable=C0116 before the first and a # pylint: enable=C0116 after the last.

By applying these rules, we reduce the occurance of this message in future.

Thank you for improving NeMo's documentation!

Copy link
Contributor

[🤖]: Hi @blisc 👋,

We wanted to let you know that a CICD pipeline for this PR just finished successfully

So it might be time to merge this PR or get some approvals

I'm just a bot so I'll leave it you what to do next.

//cc @pablo-garay @ko3n1g

Copy link
Collaborator

@XuesongYang XuesongYang left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you guys!

@paarthneekhara there is a nit-pick regarding cond to discuss, but not a blocker to merge the code.

@XuesongYang XuesongYang merged commit 7434bcd into main Jan 22, 2025
210 of 215 checks passed
@XuesongYang XuesongYang deleted the tts_transformer_2501 branch January 22, 2025 18:02
parthmannan pushed a commit that referenced this pull request Jan 28, 2025
* add core functions and classes of Transformer blocks.
* completed tests for all classes, and made bugfixes. Now all tests are passed after running,
`pytest -s -vvv tests/collections/tts/modules/test_transformer_2501.py`

---------

Signed-off-by: Jason <[email protected]>
Signed-off-by: blisc <[email protected]>
Signed-off-by: Xuesong Yang <[email protected]>
Signed-off-by: XuesongYang <[email protected]>
Co-authored-by: blisc <[email protected]>
Co-authored-by: Xuesong Yang <[email protected]>
Co-authored-by: XuesongYang <[email protected]>
Signed-off-by: Parth Mannan <[email protected]>
abhinavg4 pushed a commit that referenced this pull request Jan 30, 2025
* add core functions and classes of Transformer blocks.
* completed tests for all classes, and made bugfixes. Now all tests are passed after running,
`pytest -s -vvv tests/collections/tts/modules/test_transformer_2501.py`

---------

Signed-off-by: Jason <[email protected]>
Signed-off-by: blisc <[email protected]>
Signed-off-by: Xuesong Yang <[email protected]>
Signed-off-by: XuesongYang <[email protected]>
Co-authored-by: blisc <[email protected]>
Co-authored-by: Xuesong Yang <[email protected]>
Co-authored-by: XuesongYang <[email protected]>
Signed-off-by: Abhinav Garg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants