Repository navigation
Add opt-in deduplication to the MedDialog scenario - #4342
Open
azrabano23 wants to merge 1 commit into
Open
azrabano23 wants to merge 1 commit into
azrabano23 wants to merge 1 commit into
Conversation
The upstream med_dialog data contains duplicate entries: the icliniq test split has 3108 entries but only 2069 unique source texts (stanford-crfm#3746). Add a dedup argument, defaulting to False, to MedDialogScenario and get_med_dialog_spec, as requested in the review of stanford-crfm#4178. When True, examples whose source text duplicates an earlier example are dropped, keeping the first occurrence, and the run spec name gains a dedup=True suffix so deduplicated runs are distinguishable from previously published ones. The default behavior is unchanged for backwards compatibility.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The upstream med_dialog data contains duplicate entries; the icliniq test split has 3108 entries but only 2069 unique source texts (#3746, confirmed by @yifanmai).
Approach
This implements the deduplication inside HELM, following the review feedback on the previous attempt (#4178), which stalled after changes were requested:
MedDialogScenariogains adedupargument that defaults toFalse, andget_med_dialog_spec()exposes the same argument — exactly as requested in that review, so previously published runs remain reproducible bit-for-bit. Withdedup=False(the default) the emittedScenarioSpecandRunSpecare identical to today's.dedup=True, examples whosesrcduplicates an earlier example are dropped, keeping the first occurrence in the original order, and the run spec name becomesmed_dialog,subset={subset},dedup=Trueso deduplicated runs are distinguishable from existing ones — the "new version of this scenario" suggested in the issue thread.Tests
New
test_med_dialog_scenario.pywith two tests:test_med_dialog_scenario_dedup— deterministic, no network (it pre-seeds the split file, soensure_file_downloadedskips the download; runs in default CI): asserts the default keeps every entry including duplicates, anddedup=Truekeeps exactly the first occurrence of each source text in the original order, with the matching references.test_med_dialog_scenario_dedup_real_data— markedscenarios: asserts the real icliniq test split yields 3108 instances (2069 unique) by default and 2069 unique instances withdedup=True, preserving first occurrences.Note: at the time of filing, the CodaLab Worksheets REST API is returning HTTP 500 for this bundle (and for bundle queries generally), so the real-data test — and any fresh run of this scenario on main — currently cannot download the data. The counts in the real-data test are the ones reported and confirmed in #3746.
Closes #3746.