-
Notifications
You must be signed in to change notification settings - Fork 580
[Whisper] Add labels' in the whisper output #2252
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
[Whisper] Add labels' in the whisper output #2252
Conversation
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
bf35648
to
b0faf8a
Compare
recheck |
I'm not sure this will align with the text normalization elsewhere in the reference. Numerical values were previously expanded to full words. Will have to check how the OpenAI normalizer handles numeric values. |
@wu6u3tw Could you review Keith’s question and provide additional context or a possible response? |
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.
Per TF meeting, let's remove the euro, lb, and cent symbols since they are interacting strangely with VIM editor and don't appear to impact measured accuracy. Also, let's replicate this change in reference_SUT.py file.
speech2text/accuracy_eval.py
Outdated
"$", | ||
"¢", | ||
"£", | ||
"€", |
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.
Per TF meeting, let's remove the euro, lb, and cent symbols since they are interacting strangely with VIM editor and don't appear to impact measured accuracy. Also, let's replicate this change in reference_SUT.py file.
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.
Remove euro, cent, lb sign done.
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.
Re-reviewed and approved. Thanks!
b0faf8a
to
a51a425
Compare
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.
Reviewed and approved by Speech-to-text TF.
@pgmpablo157321 - This PR is approved per the TF discussion today. I think we can close it before the next WG sync (no need for wider input). |
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.
LGTM
The original text output from whisper-large-v3 includes numbers and the normalization part are included in the accuracy_eval script.
Therefore, to get the digit's part of the output in the label dict. I add digits, some symbols in the labels.