Move files with optional dependencies to integrations folder by pplantinga · Pull Request #2782 · speechbrain/speechbrain

pplantinga · 2024-12-10T00:15:31Z

In internal discussions, we decided to move as much of our external-dependent code as possible into a single folder for a couple of reasons:

so we could add disclaimers that this stuff could stop working at any time
additional testing requirements for code placed in this part of the repo, so we know if it stops working
testing is only run before each release, alongside recipe tests, rather than for every CI build
A readme and requirements file for using each integration

The basic concept is that we will add a python module speechbrain.integrations that contains any speechbrain blocks that depend on external libraries. So far we have:

Deprecations:

speechbrain.lobes.models.fairseq_wav2vec
speechbrain.utils.kmeans

After this approach is verified, I will:

Find and replace all deprecated references to the corresponding new paths
Update path for any integrations that I've missed, e.g. kmeans, audio tokenizers (now merged)
Add documentation to explain how to add to this folder, testing requirements, etc.

We need to ensure all integrated third-party folders have at least one doctest. Still needed:

spacyNLP
flairNLP

poonehmousavi · 2024-12-10T01:11:13Z

Thanks @pplantinga … this change would be really helpful in adding new tokenizers. Following are also needed to be transferred to integration folder:

pplantinga · 2024-12-10T01:30:40Z

Thanks @pplantinga … this change would be really helpful in adding new tokenizers. Following are also needed to be transferred to integration folder:

https://github.com/speechbrain/speechbrain/tree/develop/speechbrain/lobes/models/discrete

https://github.com/speechbrain/speechbrain/blob/develop/speechbrain/lobes/models/beats.py

I've added discrete to the list above, but I don't see any external imports in beats.py, does this code depend on an external library? IMO if the code is now internal (even if the original code was not) we don't need to put it in integrations

pplantinga · 2024-12-10T01:49:42Z

I've added discrete to the list above, but I don't see any external imports in beats.py, does this code depend on an external library? IMO if the code is now internal (even if the original code was not) we don't need to put it in integrations

In addition, are we keeping a folder for discrete in lobes.models? This makes the redirect a bit more complex.

poonehmousavi · 2024-12-11T06:20:01Z

Thanks @pplantinga … this change would be really helpful in adding new tokenizers. Following are also needed to be transferred to integration folder:

https://github.com/speechbrain/speechbrain/tree/develop/speechbrain/lobes/models/discrete

https://github.com/speechbrain/speechbrain/blob/develop/speechbrain/lobes/models/beats.py

I've added discrete to the list above, but I don't see any external imports in beats.py, does this code depend on an external library? IMO if the code is now internal (even if the original code was not) we don't need to put it in integrations

Actually, I transferred all their code to SB so it not dependant on external library, we just initialized the weights with their checkpoints

Adel-Moumen

Hi @pplantinga, the changes looks good to me. I think you also need to move Flair and spacy folder since they also depend on an external library. I would also advocate for having a README.md in each subfolder within integrations to explain how to install and make the dep and speechbrain working together (e.g. specifying the compatible versions). Finally, I would say that we should also discuss about the folders lobes and the naming conventions. I tend to think that we could increase the readability by having better names and maybe even subfolders? Idk. :)

pplantinga · 2024-12-11T16:09:05Z

Hi @pplantinga, the changes looks good to me. I think you also need to move Flair and spacy folder since they also depend on an external library.

Added to list above

I would also advocate for having a README.md in each subfolder within integrations to explain how to install and make the dep and speechbrain working together (e.g. specifying the compatible versions).

This could make good sense, we could have the requirements file that pinned a specific working version perhaps, and an author or person to contact if it stops working.

Finally, I would say that we should also discuss about the folders lobes and the naming conventions. I tend to think that we could increase the readability by having better names and maybe even subfolders? Idk. :)

What naming conventions would you change? The "lobes" name itself?

pplantinga · 2024-12-11T22:33:56Z

One more question for the crew, up to this point, we've been ignoring the CI tests for the integrations because it adds to the CI time and increases the likelihood that the CI breaks because we have to install a bunch of extra packages (and I'm wondering if some of them might not work on the CI server anyway). Do we want to add these tests back in, or is this something we want to continue skipping and just say that integrations are at your own risk. @TParcollet etc.

pplantinga · 2024-12-11T22:49:04Z

There is a long-standing issue with fairseq in newer Python versions (2.11+) that doesn't seem likely to get addressed anytime soon. Should we just go ahead and deprecate our integration?

facebookresearch/fairseq#5012

…sts pass

TParcollet · 2025-01-15T09:40:07Z

"speechbrain.nnet.losses.transducer => speechbrain.integrations.transducer ??? Should we deprecate in favor of e.g. torchaudio?" --> No, torchaudio loss is crazily slower than our Numba impl. For the tests, i'd be in favor of having them, but it sounds like a nightmare to maintain. At the same time, we provide this folder as a main part of the library... so our users may expect it to work properly. Could we have a specific CI for this folder? Like one that is only triggered if the code in it is touched?

Coverage that we should improve (I think anything bellow 60% is not really acceptable...) and since we are changing everything now, maybe good to put this in that PR?:
speechbrain/integrations/lm/ken.py
speechbrain/integrations/processing/diarization.py
speechbrain/integrations/k2_fsa/utils.py
speechbrain/integrations/k2_fsa/lattice_decoder.py
speechbrain/integrations/huggingface/wordemb/transformer.py
speechbrain/integrations/huggingface/llama2.py
speechbrain/integrations/k2_fsa/graph_compiler.py

TParcollet · 2025-01-15T09:43:40Z

Also that is a big PR, reviewing / testing will be slow.

pplantinga · 2025-01-16T20:02:50Z

Also that is a big PR, reviewing / testing will be slow.

You are right, its big but perhaps not as big as it looks since most of the changes are to module path names. For reviewing I would recommend just taking a look around the new integrations folder and seeing if the organization / readmes make sense. Some testing should be done too but again the functional changes are mostly just to module path names which should be unlikely to break anything. Biggest risk I guess is that I somehow put the wrong path for something, which recipe tests should catch.

Adel-Moumen · 2025-05-19T20:39:40Z

rather than discrete, maybe we could have been more specific i.e. audio_tokenizers or something like that.

Adel-Moumen · 2025-05-19T20:40:19Z

i think keeping the full name is better no? speech_tokenizer.py or speechtokenizer.py

We can't keep the same name as the imported library as this leads to some sort of import error. But perhaps there's a better option than speechtok.py like speechtokenizer_interface.py or something.

Yeah I understand now. I know that ESPNet is using the convention name tokenizer. E.g. from espnet.espnet2.speechlm.tokenizer import X The only issue with this is that tokenizer is a broad term and can be applied to anything. I am honestly not against having a folder that can have audio/speech tokenizers, as well as text (if one day this happen) or vision tokenizers.

Btw, shouldn't sentencepiece be moved in the .integrations. folder ?

Well currently sentencepiece is a non-optional dependency, but integrations is only for optional dependencies. I think we use it enough throughout the toolkit that perhaps its worth keeping it that way.

I understand!

Adel-Moumen · 2025-05-19T20:40:33Z

same for this one wavtokenizer or wav_tokenizer.py

wavtokenizer_interface.py ?

what about speechbrain.integrations.tokenizer.wavtokenizer ?

Adel-Moumen · 2025-05-19T20:42:34Z

shouldn't this file go to discrete folder instead? The model doesn't seems to use the HFTransformersInterface abstract class that we have in huggingface.py. It seems to be more an orchestrator of different pre-trained blocks.

Ah, good idea!

Adel-Moumen · 2025-05-19T20:44:07Z

dac or DAC ?

I think the Python standard for file names is all lower case -- due to different operating systems treating upper/lower case differently.

yep. I guess everyone in the community of codecs knows what dac refers to. We can keep it lowercase as you suggestd :)

Adel-Moumen · 2025-05-19T20:45:25Z

Is there a specific reason why we are dropping FairSeq support? I tend to agree but maybe we should be more specific about the reason why?

FB has stopped active development of FairSeq (last released version was 2022) and there is some dependency issue with a new release of some package though I can't remember which one (cuda? pytorch?)

FB stopping FairSeq, now Torchaudio :(

pplantinga · 2025-05-20T19:52:01Z

Unfortunately the MERT huggingface model is not compatible with the latest version of transformers (see huggingface/transformers#36134). I'll set up the test with an earlier version but we may have to deprecate at some point.

pplantinga · 2025-05-21T19:40:05Z

Okay, this should be pretty much ready to go @TParcollet @Adel-Moumen

Any last comments?

Adel-Moumen

LGTM!

…rain#2782) Co-authored-by: Adel Moumen <adelmoumen.pro@gmail.com>

Move k2 and hf transformers to integrations folder

9eff872

pplantinga requested review from Adel-Moumen, TParcollet, asumagic, mravanelli and poonehmousavi December 10, 2024 00:15

pplantinga added 2 commits December 9, 2024 19:25

Add readme to integrations

f5caf08

Fix overlap of module name by changing integration from k2 to k2_fsa

f7a5f6b

Update file paths in doctest

46d20ab

pplantinga self-assigned this Dec 10, 2024

pplantinga added the correctness Functionality not objectively broken, but may be surprising or wrong e.g. regarding literature label Dec 10, 2024

Move hf word embeddings to integrations

4faebea

Adel-Moumen reviewed Dec 11, 2024

View reviewed changes

Move spacy and flair to integrations

fc8b325

pplantinga changed the title ~~Move k2 and hf transformers to integrations folder~~ Move files with optional dependencies to integrations folder Dec 12, 2024

pplantinga and others added 8 commits December 12, 2024 11:34

Move speechtokenizer and fairseq to integrations

ceef346

Add deprecation notices and redirects for speechtokenizer and fairseq

e26d22b

Ignore entire integrations folder in conftest

35a0747

Add docstrings to deprecation notice files

c597b66

Add script for running third party tests before release and ensure te…

adec2de

…sts pass

Finish fixing doctests

dfcb907

The CI gods were reading from a different conftest than the one I edited

ee11335

Aaand add kenlm

e8660bb

pplantinga and others added 3 commits January 13, 2025 19:20

Add READMEs with test results to all integrations folders

63892e1

Add one test and a README to decoders

c686844

Merge branch 'develop' into integrations

a2ef5a1

pplantinga and others added 2 commits January 16, 2025 15:02

Merge branch 'develop' into integrations

71089db

Fix paths to new kenlm

24c496c

pplantinga added this to the v1.1.0 milestone Feb 14, 2025

pplantinga added 4 commits May 19, 2025 11:26

Merge branch 'develop' into integrations

6728580

Add README for decoders

6b1c163

Add docstrings to NLP test in integrations folder

a85442d

Re-up the torch version in CI

032c9ee

Adel-Moumen reviewed May 19, 2025

View reviewed changes

pplantinga added 4 commits May 20, 2025 12:32

Rename audio tokenizers files and rerun tests

9d2195c

Add doctest for kenlm scorer

252cc7d

Merge branch 'develop' into integrations

950092b

Increase doctest coverage in alignments integration

461b539

pplantinga added 3 commits May 20, 2025 15:56

Increase doctest coverage in huggingface wordemb transformer

c9d1e81

Merge branch 'develop' into integrations

5ffde78

Update path of kmeans redirect

1d52c59

Merge branch 'develop' into integrations

b9c6ac8

pplantinga mentioned this pull request May 23, 2025

Skip whisper DL from CI/CD #2926

Closed

13 tasks

fix path

0c4d1f9

Adel-Moumen approved these changes May 27, 2025

View reviewed changes

Adel-Moumen merged commit 068a41b into speechbrain:develop May 27, 2025
5 checks passed

pplantinga mentioned this pull request May 27, 2025

Alignment with CTC ASR models powered by k2. #2772

Merged

13 tasks

pplantinga added a commit to pplantinga/speechbrain that referenced this pull request Jun 2, 2025

Move files with optional dependencies to integrations folder (speechb…

5c1351a

…rain#2782) Co-authored-by: Adel Moumen <adelmoumen.pro@gmail.com>

Conversation

pplantinga commented Dec 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poonehmousavi commented Dec 10, 2024

Uh oh!

pplantinga commented Dec 10, 2024

Uh oh!

pplantinga commented Dec 10, 2024

Uh oh!

poonehmousavi commented Dec 11, 2024

Uh oh!

Adel-Moumen left a comment

Choose a reason for hiding this comment

Uh oh!

pplantinga commented Dec 11, 2024

Uh oh!

pplantinga commented Dec 11, 2024

Uh oh!

pplantinga commented Dec 11, 2024

Uh oh!

TParcollet commented Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TParcollet commented Jan 15, 2025

Uh oh!

pplantinga commented Jan 16, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pplantinga commented May 20, 2025

Uh oh!

pplantinga commented May 21, 2025

Uh oh!

Adel-Moumen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

pplantinga commented Dec 10, 2024 •

edited

Loading

TParcollet commented Jan 15, 2025 •

edited

Loading