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

Polish Evaluation Tutorial #2212

Merged
merged 6 commits into from
Feb 24, 2022
Merged

Polish Evaluation Tutorial #2212

merged 6 commits into from
Feb 24, 2022

Conversation

brandenchan
Copy link
Contributor

@brandenchan brandenchan commented Feb 17, 2022

Proposed changes:

  • Since the evaluation tutorial has gotten quite long and complex, we should make some minor changes to improve readability
  • Added link to explain new metrics
  • Added note that integrated eval mode runs as well when isolated eval mode is engaged
  • Removed the initialize_device_settings
  • Show there is no discrepancy in retriever isolated eval

To Do

  • Reader eval discrepancy still exists

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

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

Looks good to me so far but I think the filtering of no_answers needs more explanation and I would prefer to move it to a different line (earlier).

tutorials/Tutorial5_Evaluation.py Outdated Show resolved Hide resolved
]
}
],
"outputs": [],
Copy link
Member

Choose a reason for hiding this comment

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

Here it was actually on purpose to have the output in the tutorial so that users can see the format of the Evaluation Report. I think we should keep it for that reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I have included the output from just those cells which print the Evaluation Report

Copy link
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

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

LGTM! 👍 I also did a quick test run on colab and it works without any problems. With a fast internet connection in the office it took two minutes to download cross-encoder/stsb-roberta-large but other users might need to wait much longer. We could replace it with cross-encoder/stsb-roberta-base and mention in a comment that the large model would give even better results. Your choice. :)

@brandenchan brandenchan merged commit bb107e5 into master Feb 24, 2022
@brandenchan brandenchan deleted the evaluation_docs branch February 24, 2022 16:45
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

2 participants