Skip to content

Conversation

@Adel-Moumen
Copy link
Collaborator

@Adel-Moumen Adel-Moumen commented Feb 10, 2025

What does this PR do?

This PR aims at "fixing" the issue #2820 brought by PR #2781. In the latter PR, I introduced a backward incompatible change to torchaudio.load. Indeed, specifying the backend to the load function only came in torchaudio 2.1.0. It was also available in 2.0.0 and 2.0.1 but at the cost of enabling the flag export TORCHAUDIO_USE_BACKEND_DISPATCHER=1 before importing torchaudio.

After working on a workaround, I found it slower as I had to check at each read_audio call the torchaudio function, and check if there was an argument backend to the audio function. I think, it safer to upgrade our pytorch requirements to 2.1.0 for both: torch and torchaudio. PyTorch is now at version 2.6.0 and I think it should be safe for us to slowly increase the requirements as the novelties comes to make sure that the toolkit is still working. To be honest, I don't have any clues if there's still people using speechbrain with torch<2.0.0, or if speechbrain is still actually working with a very old version of torch.

Also, with torch==2.0.0 the torch.compile was immature with many bugs (sorry PyTorch team, you made a lot of nice progress), and so latter versions of pytorch are getting better and better at handling complex cases of computation graph. So, I would be in favour of slowly increasing our torch requirements, and in this case, bumping to torch>=2.1.0.

Any comments/feedbacks @TParcollet @mravanelli, and/or @pplantinga ? :)

Before submitting
  • Did you read the contributor guideline?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Does your code adhere to project-specific code style and conventions?

PR review

Reviewer checklist
  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified
  • Confirm that the changes adhere to compatibility requirements (e.g., Python version, platform)
  • Review the self-review checklist to ensure the code is ready for review

@Adel-Moumen Adel-Moumen changed the title fix backend when not avail in torchaudio.load Bump torchaudio version + add bytes Feb 10, 2025
@TParcollet
Copy link
Collaborator

I vote for the bump, it will help for rope as well.

Copy link
Collaborator

@pplantinga pplantinga left a comment

Choose a reason for hiding this comment

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

I'm good with the version bump, minor comments can be addressed or not

Comment on lines 406 to 411
if backend not in [None, "ffmpeg", "sox", "soundfile"]:
raise ValueError(
"backend must be one of 'ffmpeg', 'sox', 'soundfile' or None"
"backend must be one of 'ffmpeg', 'sox', 'soundfile' or None",
"Available backends on your system: ",
torchaudio.list_audio_backends(),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a huge deal but this code seems repeated from above, any way to combine (e.g. unified audio backend checker function)

@TParcollet
Copy link
Collaborator

@Adel-Moumen could you fix the tests?

@Adel-Moumen Adel-Moumen marked this pull request as ready for review February 11, 2025 10:52
@Adel-Moumen Adel-Moumen self-assigned this Feb 11, 2025
Copy link
Collaborator

@TParcollet TParcollet left a comment

Choose a reason for hiding this comment

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

LGTM

@TParcollet TParcollet merged commit c436f61 into speechbrain:develop Feb 11, 2025
5 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.

3 participants