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

Generate: work around PT multinomial sampling 0 probability tokens #23088

Closed
wants to merge 2 commits into from

Conversation

gante
Copy link
Member

@gante gante commented May 1, 2023

What does this PR do?

Fixes #22979

As raised in this transformers issue and this pytorch issue, multinomial can erroneously pick 0 probability tokens. According to the reports and my own observations, the error is much more likely on CPU.

There is a high chance that a token with -inf logits is selected: in this simple example with top_k=40, it happens 0.158% of the times on CPU -- or ~50% chance that a sequence with 500 newly generated tokens to have at least one token that shouldn't be there.

This PR adds a quick-and-dirty workaround, while the PT team works in the issue: at each sample step, pick 5 candidates, and keep the first valid one. Assuming independence, the probability of having one or more forbidden token in the example above drops to ~5e-10 %.

Runtime overhead: considering distilgpt2, a small model where operations outside the model have some weight, it got 2% slower on GPU (RTX3090) and 1% slower on CPU (Ryzen 9 5950X). On larger models, the slowdown becomes negligible.

@gante gante requested a review from sgugger May 1, 2023 17:58
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented May 1, 2023

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

@sgugger
Copy link
Collaborator

sgugger commented May 1, 2023

As discussed internally, this is a regression on the PyTorch side for 2.0, so this should be fixed by PyTorch and not by us adding some overload to generate.

@gante
Copy link
Member Author

gante commented May 1, 2023

(closing because of the comment above)

@gante gante closed this May 1, 2023
@gante gante deleted the fix_22979 branch May 18, 2023 15:25
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.

transition scores can be negative infinity
3 participants