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

Fix test_eos_token_id_int_and_list_top_k_top_sampling #22826

Merged
merged 2 commits into from
Apr 18, 2023
Merged

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Apr 18, 2023

What does this PR do?

In #22204, I updated the expected value in test_eos_token_id_int_and_list_top_k_top_sampling to pass the CircleCI. However, the daily CI fails with that new value. It turns out that we need a seed that could give the same (generation) output (at minimum, the same output length) on both CPU/GPU machines. The difference is very likely coming from somehow larger numerical differences after certain generation steps.

remark

With seed 0, the output generated_tokens[0] is:

  • cpu: [ 40, 416, 79, 12, 230, 89, 231, 432, 301, 212, 933, 225, 33, 33, 846]
  • gpu: [ 40, 416, 79, 12, 230, 89, 231, 432, 301, 212, 933, 225, 476, 682, 319, 832, 873, 853, 873, 832]

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Apr 18, 2023

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

@ydshieh ydshieh requested a review from sgugger April 18, 2023 13:38
@ydshieh ydshieh marked this pull request as ready for review April 18, 2023 13:38
@ydshieh ydshieh changed the title fix Fix test_eos_token_id_int_and_list_top_k_top_sampling Apr 18, 2023
@ydshieh ydshieh merged commit 90247d3 into main Apr 18, 2023
@ydshieh ydshieh deleted the fix_my_bad branch April 18, 2023 14:04
novice03 pushed a commit to novice03/transformers that referenced this pull request Jun 23, 2023
…22826)

* fix

---------

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
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.

3 participants