Skip to content

[ASR pipeline] correct asr pipeline for seq2seq models #15541

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.

Sign up for GitHub

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

Jump to bottom
Merged
patrickvonplaten merged 1 commit into from
Feb 7, 2022
Merged

[ASR pipeline] correct asr pipeline for seq2seq models #15541

patrickvonplaten merged 1 commit into from
Feb 7, 2022

Conversation

@patrickvonplaten
Copy link
Contributor

@patrickvonplaten patrickvonplaten commented Feb 7, 2022 •
edited
Loading

What does this PR do?

Fixes the following daily tests:

FAILED tests/test_pipelines_automatic_speech_recognition.py::AutomaticSpeechRecognitionPipelineTests::test_speech_to_text_leveraged
FAILED tests/test_pipelines_automatic_speech_recognition.py::AutomaticSpeechRecognitionPipelineTests::test_torch_speech_encoder_decoder
FAILED tests/test_pipelines_automatic_speech_recognition.py::AutomaticSpeechRecognitionPipelineTests::test_xls_r_from_en
FAILED tests/test_pipelines_automatic_speech_recognition.py::AutomaticSpeechRecognitionPipelineTests::test_xls_r_to_en

and adds a fast test for asr pipeline for wav2vec2-2-... model.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@HuggingFaceDocBuilder
Copy link

HuggingFaceDocBuilder commented Feb 7, 2022 •
edited
Loading

The documentation is not available anymore as the PR was closed or merged.

@patrickvonplaten
Copy link
Contributor Author

patrickvonplaten commented Feb 7, 2022

cc @Narsil - will merge since you're on holiday at the moment.

anton-l
anton-l approved these changes Feb 7, 2022
View reviewed changes
Copy link
Member

@anton-l anton-l left a comment

Choose a reason for hiding this comment

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

LGTM 👍

src/transformers/pipelines/automatic_speech_recognition.py
inputs = model_inputs.pop("input_values")
else:
raise ValueError(
"Seq2Seq speech recognition model requires either a "
Copy link
Member

@anton-l anton-l Feb 7, 2022

Choose a reason for hiding this comment

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

Suggested change
"Seq2Seq speech recognition model requires either a "
"The speech recognition model requires either an "

We're probably talking not only about seq2seq here?

Copy link
Contributor Author

@patrickvonplaten patrickvonplaten Feb 7, 2022

Choose a reason for hiding this comment

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

It's under the if self.type == "seq2seq": condition, so yeah it's just the seq2seq speech recognition model

patil-suraj
patil-suraj approved these changes Feb 7, 2022
View reviewed changes
Copy link
Contributor

@patil-suraj patil-suraj left a comment

Choose a reason for hiding this comment

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

LGTM!

@patrickvonplaten patrickvonplaten merged commit 5f1918a into huggingface:master Feb 7, 2022
@patrickvonplaten patrickvonplaten deleted the correct_asr_pipeline branch February 7, 2022 14:35
stevhliu pushed a commit to stevhliu/transformers that referenced this pull request Feb 18, 2022
@patrickvonplaten
[ASR pipeline] correct asr pipeline for seq2seq models (huggingface#1…
3473a6b 
…5541)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anton-l anton-l anton-l approved these changes

+1 more reviewer

@patil-suraj patil-suraj patil-suraj approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@patrickvonplaten @HuggingFaceDocBuilder @anton-l @patil-suraj