-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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: multilingual midel convert to tflite get wrong token #32079
Conversation
Just a friendly reminder about PR review. If you have time, could you please take a look and provide any feedback, or if everything looks good, approve it? If you have any questions or need to make changes, please let me know. Thank you so much! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, LGTM 👍 Thank you for fixing!
@Ayaa17 Ci is red because a test needs to be updated accordingly: In a nutshell, instead of checking that the intended values are |
@gante I've understood the reason for the CI failure. Is there anything I need to help or should I wait for thx a lot! |
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
@Ayaa17 please go ahead and modify |
33ca5d6
to
dd77717
Compare
dd77717
to
dac3219
Compare
@gante , I have resolved the CI issue. Could you please review the changes again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thank you for iterating 💛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thanks for your PR @Ayaa17
…e#32079) * fix: multilingual midel convert to tflite get wrong token * fix: modify test_force_tokens_logits_processor the checking value as scores.dtype.min --------- Co-authored-by: kent.sc.hung <kent.sc.hung@benq.com> Co-authored-by: Aya <[kent831217@gmail.com]>
…e#32079) * fix: multilingual midel convert to tflite get wrong token * fix: modify test_force_tokens_logits_processor the checking value as scores.dtype.min --------- Co-authored-by: kent.sc.hung <kent.sc.hung@benq.com> Co-authored-by: Aya <[kent831217@gmail.com]>
…e#32079) * fix: multilingual midel convert to tflite get wrong token * fix: modify test_force_tokens_logits_processor the checking value as scores.dtype.min --------- Co-authored-by: kent.sc.hung <kent.sc.hung@benq.com> Co-authored-by: Aya <[kent831217@gmail.com]>
What does this PR do?
when using multilingual whisper model and covert to tflite for end device use, always get wrong token.
Like this: this sample is voice transcription to chinese(zh)
It seem like
TFForceTokensLogitsProcessor
_force_token this function may happen overflow when convertedSo change the way that new_scores retains the most negative values to avoid it
result:
convert model sample code can ref this
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
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.
@gante @Rocketknight1