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

Update Pyannote with SpeechBrain 1.0 #1659

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

Adel-Moumen
Copy link

Hello hello,

This PR aims at updating the pyannote import of speechbrain to comply with the latest release of speechbrain 1.0. Unfortunately, we (the speechbrain core team) changed the path of our pretrained models from speechbrain.pretrained to speechbrain.inference. We did not add a warning, and therefore, it is now impossible to fetch any speechbrain models using speechbrain.pretrained with speechbrain==1.0. To comply with these new changes, I propose in this PR to check whether or not the version of speechbrain is greater or equal to 1.0; if so, import using speechbrain.inference, else, import as speechbrain.pretrained. This means that pyannote can support any speechbrain version >= 0.5.4 from your requirements.

Related issues: speechbrain/speechbrain#2438

If you need more on my side, please let me know, and I will try to do my best.

Best wishes

Adel

@hbredin
Copy link
Member

hbredin commented Mar 1, 2024

Thanks for the proactive fix.

I'd rather switch to speechbrain 1.x directly.

Could you update the PR accordingly (both requirements.txt and speaker_verification.py?

@Adel-Moumen
Copy link
Author

Thanks for the proactive fix.

I'd rather switch to speechbrain 1.x directly.

Could you update the PR accordingly (both requirements.txt and speaker_verification.py?

Yes will do asap (most likely in a few fours :))

@Adel-Moumen
Copy link
Author

Should be good now. CC @hbredin :)

@hbredin
Copy link
Member

hbredin commented Mar 1, 2024

To detect future breaking changes, would you mind adding a unit test that simply loads speechbrain ECAPA model? and then I think it should be good.

@Adel-Moumen
Copy link
Author

Voila. should be good now. Let me know if you need more on my side (especially regarding precommits/tests it is not obvious to me what I should run as commands to make the CI happy). @hbredin :)

@hbredin
Copy link
Member

hbredin commented Mar 8, 2024

Looks like some tests are failing.

@Adel-Moumen
Copy link
Author

Ok one failing issue is on our side. We are no longer supporting python 3.8 and so the CI is failing on a typing issue. Currently fixing this in a speechbrain PR.

@Adel-Moumen
Copy link
Author

Hello @hbredin, so the failing test is due to the Python version supported by SpeechBrain. We officially support Python 3.9+ and have therefore dropped support for Python 3.8. The failing test is due to Python 3.8 failing on typing hints. I made a PR that has been merged in SpeechBrain, which solves the issue. However, this fix is only located on the develop branch and not the main one (the default one used when running pip install speechbrain).

So there are multiple solutions. The first one would be to bump up the CI/CD server to Python 3.9. The second one would be to directly use the SpeechBrain version located on the develop branch and not the main branch. Finally, we can push a small fix on the main branch, but I would prefer to consider options 1 or 2. If necessary, we can move on to solution 3.

What do you think?

@hbredin
Copy link
Member

hbredin commented Mar 13, 2024

What about waiting for the fix to end up in a new release?

That being said, Python 3.8 apparently only represents 3% of pyannote userbase.
I might get rid of it in next big release.

@Adel-Moumen
Copy link
Author

What about waiting for the fix to end up in a new release?

That being said, Python 3.8 apparently only represents 3% of pyannote userbase. I might get rid of it in next big release.

Hello @hbredin,

We have merged a PR that allows one to use the old .pretrained. (a warning is raised asking to modify the code) path instead of .inference.. We will try to create a new release ASAP so that Pyannote works with SpeechBrain.

@hbredin
Copy link
Member

hbredin commented Apr 10, 2024

Great. Let me know when it is released and I will update the requirements.txt.

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