-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Bump torchaudio version + add bytes #2821
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
Conversation
|
I vote for the bump, it will help for rope as well. |
pplantinga
left a comment
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.
I'm good with the version bump, minor comments can be addressed or not
speechbrain/dataio/dataio.py
Outdated
| 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(), | ||
| ) |
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.
Not a huge deal but this code seems repeated from above, any way to combine (e.g. unified audio backend checker function)
|
@Adel-Moumen could you fix the tests? |
Co-authored-by: Peter Plantinga <plantinga.peter@proton.me>
TParcollet
left a comment
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.
LGTM
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 thebackendto theloadfunction only came intorchaudio2.1.0. It was also available in2.0.0and2.0.1but at the cost of enabling the flagexport TORCHAUDIO_USE_BACKEND_DISPATCHER=1before importingtorchaudio.After working on a workaround, I found it slower as I had to check at each
read_audiocall thetorchaudiofunction, and check if there was an argumentbackendto the audio function. I think, it safer to upgrade our pytorch requirements to2.1.0for both:torchandtorchaudio. PyTorch is now at version2.6.0and 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 usingspeechbrainwithtorch<2.0.0, or ifspeechbrainis still actually working with a very old version oftorch.Also, with
torch==2.0.0thetorch.compilewas 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 ourtorchrequirements, and in this case, bumping totorch>=2.1.0.Any comments/feedbacks @TParcollet @mravanelli, and/or @pplantinga ? :)
Before submitting
PR review
Reviewer checklist