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

Fix adapter tutorial r1.19.0 #6776

Merged
merged 2 commits into from
Jun 2, 2023
Merged

Conversation

hsiehjackson
Copy link
Collaborator

What does this PR do ?

Fix adapter tutorial to compatible with latest pre-traiend FastPitch checkpoint

Collection: [TTS]

Changelog

  • Add specific line by line info of high level changes in this PR.

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this 

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

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

  • Related to # (issue)

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.

please keep the changes for notebook metadata as minimal as possible. I observed frequent changes for its cell id and python version.

out = self.gst_module(reference_spec, reference_spec_lens)
embs = out if embs is None else embs + out
else:
logging.warning("You may add `gst_module` in speaker_encoder to use reference_audio.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we relax the logging level to warning from errors? I was wondering if we should raise errors when the behavior is not expected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We previously raise error when the model has gst_module and users don't provide reference_spec.
However, there is a scenario we pre-compute the speaker embeddings of gst_module, so the model still has gst_module but we don't provide reference_spec anymore.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. it is fine for me. @rlangman @redoctopus any thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fine with me, though could you add a sentence in the warning telling users what it does by default instead?

@hsiehjackson hsiehjackson force-pushed the fix_adapter_tutorial_r1.19.0 branch 2 times, most recently from 85d4230 to d56ee88 Compare June 1, 2023 17:12
Signed-off-by: hsiehjackson <c2hsieh@ucsd.edu>
Signed-off-by: hsiehjackson <c2hsieh@ucsd.edu>
@hsiehjackson hsiehjackson merged commit 9bd8ecd into r1.19.0 Jun 2, 2023
@hsiehjackson hsiehjackson deleted the fix_adapter_tutorial_r1.19.0 branch June 2, 2023 03:55
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.

3 participants