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 Multiresolution HuBERT as an additional upstream model #517

Merged
merged 16 commits into from
Dec 11, 2023

Conversation

ftshijt
Copy link
Collaborator

@ftshijt ftshijt commented Nov 28, 2023

As per discussion in #515 ,this PR adds the multiresolution HuBERT as an additional upstream model.

TODOs:

  • docs
  • upload converted ckpt to huggingface (as well as related entry in hubconf.py)

Reference PR in fairseq:

Btw, during the evaluation of vc task, also fixed a minor bug related to new updates in librosa

@ftshijt
Copy link
Collaborator Author

ftshijt commented Dec 8, 2023

@leo19941227 Hi Leo, the PR is ready for review now (I also fix the huggingface repo for a range of pre-trained models). I will continue to add corresponding docs to the documentation page.

@leo19941227
Copy link
Member

Hi @ftshijt !

Sure, I will review the changes this weekend. Thanks so much!

@leo19941227 leo19941227 self-requested a review December 10, 2023 15:18
Comment on lines +1002 to +1038
multires_hubert_base
~~~~~~~~~~~~~~~~~~~~~

- Unlabled Speech: LibriSpeech 960hr
- K-means extracted from `hubert_base`_


multires_hubert_large
~~~~~~~~~~~~~~~~~~~~~

- Unlabeled Speech: LibriLight 60khr
- K-means extracted from `hubert_base`_


multires_hubert_multilingual_base
---------------------------------

- Unlabeled Speech: Voxpopuli 100khr
- K-means extracted from `hubert_base`_


multires_hubert_multilingual_large400k
--------------------------------------

- Unlabeled Speech: Voxpopuli 100khr
- K-means extracted from `hubert_base`_
- Training steps 400k


multires_hubert_multilingual_large600k
--------------------------------------

- Unlabeled Speech: Voxpopuli 100khr
- K-means extracted from `hubert_base`_
- Training steps 600k


Copy link
Member

Choose a reason for hiding this comment

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

For the K-means extraction source, the current document implies that all the models are using the K-means from the 2nd iteration of HuBERT Base, could you help double check if this is true?
For example, this means multires_hubert_base is the 3rd iteration of the HuBERT training, which might not be directly comparable to hubert_base.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, the current note is true. All the provided models shall be considered as the 3rd iteration of the HuBERT-training (not directly comparable to hubert_base). In our provided paper, we also conducted the experiments with the "comparable" hubert using the same 3rd iteration k-means.

In that case, do you suggest me submit those models (3rd iteration hubert) to the s3prl as well?

Copy link
Member

Choose a reason for hiding this comment

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

Oh I think adding the 3rd iteration of HuBERT is not necessary. I was just curious and making sure that the note is correct. I have no further issue then. (You can still submit the 3rd iteration HuBERT if you want)

Copy link
Member

@leo19941227 leo19941227 left a comment

Choose a reason for hiding this comment

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

Thanks @ftshijt !
I have a few comments, please help check it.
Thanks!

@ftshijt
Copy link
Collaborator Author

ftshijt commented Dec 11, 2023

Thanks @ftshijt ! I have a few comments, please help check it. Thanks!

Many thanks for the review! I've removed the unused legacy note, but I'm not sure if we would also like to add the 3rd iteration hubert-base as discussed above. But I feel even in that case, I can probably simply add that to the s3prl/hubert huggingface repo and leave it there for people who are interested in a fair comparison between hubert and mr-hubert (let me know if you have any other better ideas~).

@leo19941227
Copy link
Member

Hi @ftshijt ,

I am good with the current changes and I am ready to merge this.
Let me know if you are ready or you plan to submit the 3rd iteration HuBERT in this PR.

@ftshijt
Copy link
Collaborator Author

ftshijt commented Dec 11, 2023

Hi @ftshijt ,

I am good with the current changes and I am ready to merge this. Let me know if you are ready or you plan to submit the 3rd iteration HuBERT in this PR.

Thanks again for the review. I would prefer to merge this PR. For the 3rd iteration hubert, I will commit that to the huggingface repo directly for reference purposes.

@leo19941227
Copy link
Member

Sounds good!

@leo19941227 leo19941227 merged commit fd69afe into s3prl:main Dec 11, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants