diff --git a/lm_eval/tasks/bbh/cot_zeroshot/utils.py b/lm_eval/tasks/bbh/cot_zeroshot/utils.py index 4c4e8dad9..8ded5e6a6 100644 --- a/lm_eval/tasks/bbh/cot_zeroshot/utils.py +++ b/lm_eval/tasks/bbh/cot_zeroshot/utils.py @@ -1,4 +1,3 @@ -import collections import re import sys import unicodedata @@ -38,12 +37,17 @@ class ExtendedRegexFilter(RegexFilter): st = st.translate(self.punct_tbl) return st - def find_match(self, regex, resp, convert_dict={}): + def find_match(self, regex, resp, convert_dict: dict[str, str] | None = None): + if convert_dict is None: + convert_dict = {} match = regex.findall(resp) if match: match = match[self.group_select] if isinstance(match, tuple): - match = [m for m in match if m][0] + non_empty = [m for m in match if m] + if not non_empty: + return "" + match = non_empty[0] match = match.strip() if match and match in convert_dict: match = convert_dict[match] @@ -53,20 +57,23 @@ class ExtendedRegexFilter(RegexFilter): class MapRegexFilter(ExtendedRegexFilter): def __init__( self, - regex_pattern_to_value: dict = {}, + regex_pattern_to_value: dict | None = None, group_select=0, fallback: str = "[invalid]", ignore_case=False, ignore_punctuation=False, regexes_to_ignore=None, ) -> None: - """ + """Map regex matches to fixed values. + regex_pattern_to_value: Match the regex pattern and change the result into the value group_select: Selects the (group_select)th match from the findall result. We use the whole regex_patterns, concatenated by | ignore_case: Lowers the case of response before matching with the given regex ignore_punctuation: Remove the punctuation before matching with the given regex regexes_to_ignore: Remove these regexes before matching with the given regex """ + if regex_pattern_to_value is None: + regex_pattern_to_value = {} super().__init__( "|".join(list(regex_pattern_to_value.keys())), group_select, @@ -138,21 +145,16 @@ class NumberParseRegexFilter(ExtendedRegexFilter): class WordSortFilter(Filter): - """ """ - def apply(self, resps, docs): filtered_resps = [] - for r, doc in zip(resps, docs): + for r, doc in zip(resps, docs, strict=True): words = doc["input"].split("List:")[1].strip().split() regex = re.compile("|".join([f"\\b{w}\\b" for w in words])) filtered = [] for resp in r: match = regex.findall(resp) - match.reverse() - ordered_words = reversed( - collections.OrderedDict(zip(match, [None] * len(match))) - ) + ordered_words = reversed(dict.fromkeys(reversed(match))) filtered.append(" ".join(ordered_words)) filtered_resps.append(filtered) @@ -161,7 +163,8 @@ class WordSortFilter(Filter): class MultiChoiceRegexFilter(ExtendedRegexFilter): def __init__(self, *args, **kwargs): - r""" + r"""Extract a multiple-choice answer letter from a response. + regex_pattern: The basic regex pattern to use. If fails to match, we will use the customized match procedure - step 1 : We parse the choices between ([A-Z])s then try to find these choices in the response. - step 2 : We parse the choice with regex :[\s]*([A-?]), where ? varies by number of choices. @@ -180,7 +183,7 @@ class MultiChoiceRegexFilter(ExtendedRegexFilter): filtered_resps = [] - for r, doc in zip(resps, docs): + for r, doc in zip(resps, docs, strict=False): fallback_regexes = [] choice_to_alpha = {} next_alpha = "A" diff --git a/lm_eval/tasks/bbh/zeroshot/utils.py b/lm_eval/tasks/bbh/zeroshot/utils.py index 4c4e8dad9..cedc6b291 100644 --- a/lm_eval/tasks/bbh/zeroshot/utils.py +++ b/lm_eval/tasks/bbh/zeroshot/utils.py @@ -1,4 +1,3 @@ -import collections import re import sys import unicodedata @@ -38,12 +37,17 @@ class ExtendedRegexFilter(RegexFilter): st = st.translate(self.punct_tbl) return st - def find_match(self, regex, resp, convert_dict={}): + def find_match(self, regex, resp, convert_dict: dict[str, str] | None = None): + if convert_dict is None: + convert_dict = {} match = regex.findall(resp) if match: match = match[self.group_select] if isinstance(match, tuple): - match = [m for m in match if m][0] + non_empty = [m for m in match if m] + if not non_empty: + return "" + match = non_empty[0] match = match.strip() if match and match in convert_dict: match = convert_dict[match] @@ -53,20 +57,23 @@ class ExtendedRegexFilter(RegexFilter): class MapRegexFilter(ExtendedRegexFilter): def __init__( self, - regex_pattern_to_value: dict = {}, + regex_pattern_to_value: dict | None = None, group_select=0, fallback: str = "[invalid]", ignore_case=False, ignore_punctuation=False, regexes_to_ignore=None, ) -> None: - """ + """Map regex matches to fixed values. + regex_pattern_to_value: Match the regex pattern and change the result into the value group_select: Selects the (group_select)th match from the findall result. We use the whole regex_patterns, concatenated by | ignore_case: Lowers the case of response before matching with the given regex ignore_punctuation: Remove the punctuation before matching with the given regex regexes_to_ignore: Remove these regexes before matching with the given regex """ + if regex_pattern_to_value is None: + regex_pattern_to_value = {} super().__init__( "|".join(list(regex_pattern_to_value.keys())), group_select, @@ -138,21 +145,16 @@ class NumberParseRegexFilter(ExtendedRegexFilter): class WordSortFilter(Filter): - """ """ - def apply(self, resps, docs): filtered_resps = [] - for r, doc in zip(resps, docs): + for r, doc in zip(resps, docs, strict=True): words = doc["input"].split("List:")[1].strip().split() regex = re.compile("|".join([f"\\b{w}\\b" for w in words])) filtered = [] for resp in r: match = regex.findall(resp) - match.reverse() - ordered_words = reversed( - collections.OrderedDict(zip(match, [None] * len(match))) - ) + ordered_words = reversed(dict.fromkeys(reversed(match))) filtered.append(" ".join(ordered_words)) filtered_resps.append(filtered) @@ -161,7 +163,8 @@ class WordSortFilter(Filter): class MultiChoiceRegexFilter(ExtendedRegexFilter): def __init__(self, *args, **kwargs): - r""" + r"""Extract a multiple-choice answer letter from a response. + regex_pattern: The basic regex pattern to use. If fails to match, we will use the customized match procedure - step 1 : We parse the choices between ([A-Z])s then try to find these choices in the response. - step 2 : We parse the choice with regex :[\s]*([A-?]), where ? varies by number of choices. @@ -180,7 +183,7 @@ class MultiChoiceRegexFilter(ExtendedRegexFilter): filtered_resps = [] - for r, doc in zip(resps, docs): + for r, doc in zip(resps, docs, strict=True): fallback_regexes = [] choice_to_alpha = {} next_alpha = "A" diff --git a/tests/test_bbh_filters.py b/tests/test_bbh_filters.py new file mode 100644 index 000000000..d9d359157 --- /dev/null +++ b/tests/test_bbh_filters.py @@ -0,0 +1,74 @@ +import re + +import pytest + +from lm_eval.tasks.bbh.cot_zeroshot.utils import ( + ExtendedRegexFilter as CotZeroshotRegexFilter, + MapRegexFilter as CotZeroshotMapFilter, +) +from lm_eval.tasks.bbh.zeroshot.utils import ( + ExtendedRegexFilter as ZeroshotRegexFilter, + MapRegexFilter as ZeroshotMapFilter, +) + + +# `bbh/zeroshot/utils.py` and `bbh/cot_zeroshot/utils.py` are byte-identical +# copies of the same filters, so every case below runs against both. +REGEX_FILTERS = [ + pytest.param(ZeroshotRegexFilter, id="zeroshot"), + pytest.param(CotZeroshotRegexFilter, id="cot_zeroshot"), +] +MAP_FILTERS = [ + pytest.param(ZeroshotMapFilter, id="zeroshot"), + pytest.param(CotZeroshotMapFilter, id="cot_zeroshot"), +] + +# Both groups are optional and the matched text ("x") lies outside them, so +# `findall` yields a tuple whose every element is the empty string. +OPTIONAL_GROUPS = re.compile(r"x(foo)?(bar)?") + + +@pytest.mark.parametrize("filter_cls", REGEX_FILTERS) +def test_all_empty_tuple_returns_empty_string(filter_cls): + """An all-empty capture tuple is a non-match, not an IndexError. + + `[m for m in match if m][0]` indexed an empty list here, so a regex with two + optional groups crashed the run instead of falling through to the fallback. + Mirrors the guard in lm_eval/filters/extraction.py. + """ + assert OPTIONAL_GROUPS.findall("x") == [("", "")] + assert filter_cls().find_match(OPTIONAL_GROUPS, "x") == "" + + +@pytest.mark.parametrize("filter_cls", REGEX_FILTERS) +def test_partially_empty_tuple_takes_first_non_empty_group(filter_cls): + """The surviving group is still selected when only some groups are empty.""" + assert OPTIONAL_GROUPS.findall("xbar") == [("", "bar")] + assert filter_cls().find_match(OPTIONAL_GROUPS, "xbar") == "bar" + + +@pytest.mark.parametrize("filter_cls", REGEX_FILTERS) +def test_single_group_match_is_unchanged(filter_cls): + """The common path -- one capture group, so `findall` returns plain strings.""" + regex = re.compile(r"\(([A-Z])\)") + assert filter_cls().find_match(regex, "The answer is (B)") == "B" + + +@pytest.mark.parametrize("filter_cls", REGEX_FILTERS) +def test_convert_dict_still_applied(filter_cls): + """A matched value is still mapped through `convert_dict`.""" + regex = re.compile(r"\(([A-Z])\)") + assert filter_cls().find_match(regex, "(B)", {"B": "(B)"}) == "(B)" + + +@pytest.mark.parametrize("filter_cls", MAP_FILTERS) +def test_map_filter_falls_back_instead_of_crashing(filter_cls): + """End to end: an all-empty tuple yields the fallback, not a crash. + + MapRegexFilter joins its configured patterns with "|", which is how a + multi-group regex reaches `find_match` in the first place. + """ + filter_instance = filter_cls( + regex_pattern_to_value={r"x(foo)?": "X", r"y(bar)?": "Y"} + ) + assert filter_instance.apply([["x"]], [{}]) == [["[invalid]"]]