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

[TTS] Fix audio codec type checks #7373

Merged
merged 2 commits into from
Sep 19, 2023
Merged

[TTS] Fix audio codec type checks #7373

merged 2 commits into from
Sep 19, 2023

Conversation

rlangman
Copy link
Collaborator

@rlangman rlangman commented Sep 5, 2023

What does this PR do ?

Most classes in the audio codec code had input_type() and output_type() parameters but were missing the typecheck() annotation on the forward() method to enforce them.

This commit adds the typecheck() to all relevant forward() methods and fixes their corresponding type annotations.

Also has a small change to add the vector quantizer parameters to the optimizer in the model (though the existing RVQ has no trainable parameters).

Collection: [TTS]

Changelog

  • Add typecheck to forward methods
  • Fix various typos in input/output types

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation?
  • Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
    • Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

  • New Feature
  • Bugfix
  • Documentation

nithinraok
nithinraok previously approved these changes Sep 6, 2023
Copy link
Collaborator

@nithinraok nithinraok left a comment

Choose a reason for hiding this comment

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

LGTM

@nithinraok
Copy link
Collaborator

Are they no test cases for Codecs yet?

@rlangman
Copy link
Collaborator Author

rlangman commented Sep 6, 2023

Are they no test cases for Codecs yet?

There are only a few. We haven't added CI tests which run the recipe itself, and I could not think of many tests that were not effectively just asserting what the neural typechecking is supposed to validate automatically at runtime in terms of types/dimensions of each module (though clearly it wasn't being validated).

@titu1994
Copy link
Collaborator

titu1994 commented Sep 6, 2023

Type checks work if the entire pipeline of types is setup - including data loader and loss.

anteju
anteju previously approved these changes Sep 7, 2023
Copy link
Collaborator

@anteju anteju left a comment

Choose a reason for hiding this comment

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

LGTM

@rlangman rlangman force-pushed the codec_typecheck branch 2 times, most recently from 3402183 to 53e12b7 Compare September 8, 2023 23:50
Signed-off-by: Ryan <rlangman@nvidia.com>
Signed-off-by: Ryan <rlangman@nvidia.com>
@rlangman rlangman dismissed stale reviews from anteju and nithinraok via 97ee70f September 11, 2023 14:12
@rlangman rlangman requested a review from anteju September 19, 2023 15:47
@rlangman rlangman merged commit 43c93d8 into main Sep 19, 2023
@rlangman rlangman deleted the codec_typecheck branch September 19, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants