-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[SVS] VISinger Plus #5741
[SVS] VISinger Plus #5741
Conversation
merge master to hubert
merge from master to hubert
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.
Many thanks for the great work. I left a few comments to further improve it~
egs2/ofuton_p_utagoe_db/svs1/conf/tuning/train_visinger_hubert.yaml
Outdated
Show resolved
Hide resolved
egs2/opencpop/svs1/conf/tuning/train_visinger_hubert_cn_mert.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Jiatong <[email protected]>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
I have resolved all comments. Please review when you have time @ftshijt, many thanks! |
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.
Many thanks for your contribution!
A few minor comments as below. Also, it would be super helpful if you could add unit tests along with the new setup.
if torch.cuda.is_available(): | ||
dev = "cuda" | ||
else: | ||
dev = "cpu" |
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.
Since it is a sub-module of the gan_svs model, we may not need to manually add the device setup?
After fixing the issue, we can safely merge the PR. |
Also, please fix the import CI error in the import test. |
Co-authored-by: Jiatong <[email protected]>
Co-authored-by: Jiatong <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5741 +/- ##
===========================================
+ Coverage 32.51% 54.27% +21.76%
===========================================
Files 768 767 -1
Lines 70372 70338 -34
===========================================
+ Hits 22878 38173 +15295
+ Misses 47494 32165 -15329
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Many thanks for the update and your great contribution! I will merge the PR, but please remember to submit another PR for CI test in the second stage. |
What?
Use pre-trained SSL models as additional input information for VISinger 2.
Why?
SVS enhancement.
See also
@ftshijt