asr model evaluator addition + doc#378
Conversation
|
The documentation is not available anymore as the PR was closed or merged. |
lvwerra
left a comment
There was a problem hiding this comment.
Hi @bayartsogt-ya, thank you for this incredibly clean PR! Just one small nit and I'll fix the output format of WER/CER.
| """ | ||
| Examples: | ||
| ```python | ||
| >>> from evaluate import evaluator | ||
| >>> from datasets import load_dataset | ||
| >>> task_evaluator = evaluator("automatic-speech-recognition") | ||
| >>> data = load_dataset("mozilla-foundation/common_voice_11_0", "en", split="validation[:40]") | ||
| >>> results = task_evaluator.compute( | ||
| >>> model_or_pipeline="https://huggingface.co/openai/whisper-tiny.en", | ||
| >>> data=data, | ||
| >>> input_column="path", | ||
| >>> label_column="sentence", | ||
| >>> metric="wer", | ||
| >>> ) | ||
| ``` | ||
| """ |
There was a problem hiding this comment.
For the other Evaluators we added the example also to a string at the beginning that we then added with @add_end_docstrings. Could we do the same here for uniformity?
| random_state=random_state, | ||
| ) | ||
|
|
||
| # TODO: To clarify why `wer` and `cer` return float |
There was a problem hiding this comment.
That's an oversight in the standardization of the metrics. Let me fix that in a separate PR so we can remove this here.
There was a problem hiding this comment.
Actually that will probably break a few things so let's keep your workaround for now.
There was a problem hiding this comment.
This sounds great and thanks for making this change! Let me merge with your branch and do changes accordingly.
There was a problem hiding this comment.
Let's actually keep your workaround for now.
|
Hi @bayartsogt-ya, see my comment above: can you keep your workaround and just fix the docstring. also no need to merge the upstream branch into yours then. Thanks! |
4d5eba7 to
3bd2788
Compare
|
Awesome, thanks for this addition! |
@lewtun
As discussed #324, I am adding
automatic-speech-recognitionevaluator here.Since
automatic-speech-recognitionpipeline already exists and it supports bothWav2Vec2andWhisper, I think it is safe addition.I did have some concern where
werandcermetrics both returningfloatwhile contract says it should bedict.So I kind of hacked the way around by checking the type, but definitely want to hear your opinion on this.
Thanks!