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.

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

Docs fix: Multinomial sampling decoding needs "num_beams=1", since by default it is usually not 1. #22473

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Docs fix: Multinomial sampling decoding needs "num_beams=1", since by default it is usually not 1. #22473

merged 1 commit into from
Mar 30, 2023

Conversation

manueldeprada
Copy link
Contributor

Fix error in docs: multinomial sampling decoding strategy

As indicated in the library source code:
https://github.com/huggingface/transformers/blob/228792a9dc0c36f1e82ab441e1b1991d116ee0a0/src/transformers/generation/utils.py#LL1364-L1367

Multinomial sampling needs num_beams=1. However, this is not indicated in the docs, potentially leading to execute beam-search multinomial sampling instead of the intended multinomial sampling.

This deviation from the expected behaviour happens quite often, since a lot of models have in their generation_config.json the parameter num_beams set to something higher than 1. This happens, for example, in the majority of top translation models from the Hub.

Also, I have included "ancestral sampling" as another name for multinomial sampling, since it is the most common name in the decoding algorithms literature.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).

Who can review?

Original authors of this piece of documentation: @gante, @sgugger, @stevhliu and @MKhalusova

@manueldeprada manueldeprada changed the title Docs fix: Multinomial sampling dedoding needs "num_beams=1", since by default it is usually not 1. Docs fix: Multinomial sampling decoding needs "num_beams=1", since by default it is usually not 1. Mar 30, 2023
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Mar 30, 2023

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

@MKhalusova MKhalusova requested a review from gante March 30, 2023 13:34
Copy link
Member

@gante gante left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the correction!

@gante gante requested review from gante and sgugger March 30, 2023 15:00
@sgugger sgugger merged commit d5de578 into huggingface:main Mar 30, 2023
raghavanone pushed a commit to raghavanone/transformers that referenced this pull request Apr 5, 2023
… default it is usually not 1. (huggingface#22473)

Fix: Multinomial sampling needs "num_beams=1", since by default is 5.
novice03 pushed a commit to novice03/transformers that referenced this pull request Jun 23, 2023
… default it is usually not 1. (huggingface#22473)

Fix: Multinomial sampling needs "num_beams=1", since by default is 5.
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.

4 participants