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.

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

Make ASR pipeline compliant with Hub spec + add tests #33769

Merged
merged 8 commits into from
Oct 1, 2024
Merged

Make ASR pipeline compliant with Hub spec + add tests #33769

merged 8 commits into from
Oct 1, 2024

Conversation

Rocketknight1
Copy link
Member

@Rocketknight1 Rocketknight1 commented Sep 27, 2024 •
edited
Loading

As well as making the ASR pipeline compliant, this PR also refactors out the output test, so we can just import that in future PRs as well.

@Rocketknight1
Copy link
Member Author

cc @LysandreJik @osanseviero @Wauplin - there will be quite a lot of these if I do pipelines individually, maybe I should batch them into PRs with a few pipelines each?

vignesh1507
vignesh1507 suggested changes Sep 27, 2024
View reviewed changes
Copy link

@vignesh1507 vignesh1507 left a comment

Choose a reason for hiding this comment

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

I agree with the changes, could you elaborate more about the changes?

@HuggingFaceDocBuilderDev
Copy link

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Wauplin
Wauplin approved these changes Sep 30, 2024
View reviewed changes
Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Thanks @Rocketknight1!
I'm not against some batching now that the main helpers are implemented. But note that I'm not a transformers maintainers so my approval is mehh 😄

@LysandreJik
Copy link
Member

@vignesh1507 you've been posting this message on a few PRs; please explain exactly what you mean as it feels like spam. Thank you.

LysandreJik
LysandreJik approved these changes Oct 1, 2024
View reviewed changes
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

🔥

src/transformers/pipelines/automatic_speech_recognition.py Outdated Show resolved Hide resolved
@LysandreJik
Copy link
Member

I don't feel strongly about batching PRs, if it makes things simpler then let's do it yeah

@Rocketknight1 @LysandreJik
Update src/transformers/pipelines/automatic_speech_recognition.py
5b4d8d7 
Co-authored-by: Lysandre Debut <hi@lysand.re>
@HuggingFaceDocBuilderDev
Copy link

Hey! 🤗 Thanks for your contribution to the transformers library!

Before merging this pull request, slow tests CI should be triggered. To enable this:

  • Add the run-slow label to the PR
  • When your PR is ready for merge and all reviewers' comments have been addressed, push an empty commit with the command [run-slow] followed by a comma separated list of all the models to be tested, i.e. [run_slow] model_to_test_1, model_to_test_2
    • If the pull request affects a lot of models, put at most 10 models in the commit message
  • A transformers maintainer will then approve the workflow to start the tests

(For maintainers) The documentation for slow tests CI on PRs is here.

@Rocketknight1
Copy link
Member Author

I'll batch simpler/related ones, in that case, which should save time without making it overwhelming to review.

@Rocketknight1 Rocketknight1 merged commit a43e84c into main Oct 1, 2024
25 checks passed
@Rocketknight1 Rocketknight1 deleted the asr_pipeline_spec_compliance branch October 1, 2024 17:15
@Rocketknight1 Rocketknight1 mentioned this pull request Oct 10, 2024
Merged
5 tasks
NielsRogge pushed a commit to NielsRogge/transformers that referenced this pull request Oct 21, 2024
@Rocketknight1 @LysandreJik @NielsRogge
Make ASR pipeline compliant with Hub spec + add tests (huggingface#33769
7a0a618 
)

* Remove max_new_tokens arg

* Add ASR pipeline to testing

* make fixup

* Factor the output test out into a util

* Full error reporting

* Full error reporting

* Update src/transformers/pipelines/automatic_speech_recognition.py

Co-authored-by: Lysandre Debut <hi@lysand.re>

* Small comment

---------

Co-authored-by: Lysandre Debut <hi@lysand.re>
BernardZach pushed a commit to BernardZach/transformers that referenced this pull request Dec 5, 2024
@Rocketknight1 @LysandreJik @BernardZach
Make ASR pipeline compliant with Hub spec + add tests (huggingface#33769
984ec0f 
)

* Remove max_new_tokens arg

* Add ASR pipeline to testing

* make fixup

* Factor the output test out into a util

* Full error reporting

* Full error reporting

* Update src/transformers/pipelines/automatic_speech_recognition.py

Co-authored-by: Lysandre Debut <hi@lysand.re>

* Small comment

---------

Co-authored-by: Lysandre Debut <hi@lysand.re>
BernardZach pushed a commit to innovationcore/transformers that referenced this pull request Dec 6, 2024
@Rocketknight1 @LysandreJik @BernardZach
Make ASR pipeline compliant with Hub spec + add tests (huggingface#33769
bfb85a8 
)

* Remove max_new_tokens arg

* Add ASR pipeline to testing

* make fixup

* Factor the output test out into a util

* Full error reporting

* Full error reporting

* Update src/transformers/pipelines/automatic_speech_recognition.py

Co-authored-by: Lysandre Debut <hi@lysand.re>

* Small comment

---------

Co-authored-by: Lysandre Debut <hi@lysand.re>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vignesh1507 vignesh1507 vignesh1507 requested changes

@LysandreJik LysandreJik LysandreJik approved these changes

@Wauplin Wauplin Wauplin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Rocketknight1 @HuggingFaceDocBuilderDev @LysandreJik @Wauplin @vignesh1507