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

Add QuestionAnsweringEvaluator #179

Merged
merged 23 commits into from
Jul 19, 2022

Conversation

fxmarty
Copy link
Contributor

@fxmarty fxmarty commented Jul 6, 2022

This PR adds a subclass for Evaluator to handle question-answering. I kept label_column for the answers, but I wonder if we should not rename it everywhere reference_column in all evaluators, not sure.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jul 6, 2022

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

Copy link
Member

@lvwerra lvwerra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @fxmarty for adding this - it is in really good shape already. Just a few minor comments.

src/evaluate/evaluator/question_answering.py Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Show resolved Hide resolved
@lvwerra
Copy link
Member

lvwerra commented Jul 7, 2022

After adding these evaluators we should really work on adding a bit of documentation for them!

@fxmarty fxmarty force-pushed the add-question-answering-evaluator branch from 1096dc7 to f0e11ff Compare July 7, 2022 14:04
src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
@fxmarty fxmarty force-pushed the add-question-answering-evaluator branch from 540956f to 6aa0a2f Compare July 13, 2022 08:22
Copy link
Member

@lvwerra lvwerra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. Left two comments/questions.

src/evaluate/evaluator/question_answering.py Outdated Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Show resolved Hide resolved
@ola13 ola13 mentioned this pull request Jul 14, 2022
@fxmarty fxmarty force-pushed the add-question-answering-evaluator branch from fb7fffc to 56d74bb Compare July 15, 2022 13:29
@fxmarty
Copy link
Contributor Author

fxmarty commented Jul 15, 2022

I refactored a bit according to #185 .

Copy link
Member

@lvwerra lvwerra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! If we can resolve the few open comments plus the few new ones I left we can merge :)

src/evaluate/evaluator/question_answering.py Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Show resolved Hide resolved
src/evaluate/evaluator/question_answering.py Show resolved Hide resolved
@fxmarty fxmarty force-pushed the add-question-answering-evaluator branch from 96556c2 to 665df6f Compare July 18, 2022 08:28
@lvwerra lvwerra merged commit 441b6e3 into huggingface:main Jul 19, 2022
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.

None yet

4 participants