Skip to content

Add GEM/xsum prompts #745

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

Merged
merged 3 commits into from
Apr 27, 2022
Merged

Conversation

kkawamu1
Copy link

Add 10 prompts for GEM/xsum.
All the prompts are taken from the prompts already implemented for https://huggingface.co/datasets/xsum in the host version promptsource.

@kkawamu1 kkawamu1 changed the base branch from main to eval-hackathon April 26, 2022 21:38
@jzf2101 jzf2101 self-requested a review April 26, 2022 22:18
@jzf2101 jzf2101 self-assigned this Apr 26, 2022
@jzf2101
Copy link
Collaborator

jzf2101 commented Apr 27, 2022

@awebson from an engineering perspective do we have to copy these directly or could we pull from https://huggingface.co/datasets/xsum as is?

@jzf2101 jzf2101 requested a review from awebson April 27, 2022 04:56
@kkawamu1
Copy link
Author

@awebson from an engineering perspective do we have to copy these directly or could we pull from https://huggingface.co/datasets/xsum as is?

There is actually a slight difference in that https://huggingface.co/datasets/xsum uses 'summary' as a key for the summary text, while GEM/xsum uses 'target' as a key. So the prompts written for GEM/xsum reflect that difference.

e.g.

{{document}} This boils down to the simple idea that ||| {{target}}

instead of

{{document}} This boils down to the simple idea that ||| {{summary}}

Copy link
Contributor

@awebson awebson left a comment

Choose a reason for hiding this comment

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

Yup what Ken did is correct. We need to copy the old xsum prompts to gem/xsum and make necessary changes. Merging now since the xsum prompts were already approved. Thanks!

@@ -26,7 +26,7 @@

# These are users whose datasets should be included in the results returned by
# filter_english_datasets (regardless of their metadata)
INCLUDED_USERS = {"Zaid", "craffel"}
INCLUDED_USERS = {"Zaid", "craffel", "GEM"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
INCLUDED_USERS = {"Zaid", "craffel", "GEM"}
INCLUDED_USERS = {"Zaid", "craffel", "GEM"}

@awebson
Copy link
Contributor

awebson commented Apr 27, 2022

@kkawamu1 Looks like taking out the hack broke automatic tests :( And I somehow I don't have permission to commit a fix from my end. Can you re-add GEM to INCLUDED_USERS = {"Zaid", "craffel", "GEM"} and push again?

@awebson awebson assigned awebson and unassigned jzf2101 Apr 27, 2022
@kkawamu1
Copy link
Author

kkawamu1 commented Apr 27, 2022

@awebson Should be updated! i.e. GEM is added back to INCLUDED_USERS now!

@awebson awebson merged commit 06bd60d into bigscience-workshop:eval-hackathon Apr 27, 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.

3 participants