Skip to content

Commit 40d477a

Browse files
authored
Merge pull request #830 from qiyanjun/fix/issue-triage-quick-fixes
Fix bert-attack slowness, CLARE <SPLIT>-token corruption, and WordSwapInflections POS mismatch
2 parents 90e5e5a + dd850e5 commit 40d477a

6 files changed

Lines changed: 164 additions & 14 deletions

File tree

tests/test_attacked_text.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,16 @@ def attacked_text_pair():
4848
return textattack.shared.AttackedText(raw_text_pair)
4949

5050

51+
raw_text_pair_split_word_collision = collections.OrderedDict(
52+
[("premise", "hello there"), ("hypothesis", "I am fine.")]
53+
)
54+
55+
56+
@pytest.fixture
57+
def attacked_text_pair_split_word_collision():
58+
return textattack.shared.AttackedText(raw_text_pair_split_word_collision)
59+
60+
5161
class TestAttackedText:
5262
def test_words(self, attacked_text, pokemon_attacked_text):
5363
# fmt: off
@@ -144,6 +154,18 @@ def test_word_insertion(self, attacked_text):
144154
== "A person walks a long way up stairs into a room and sees beer poured from a keg and people on the couch talking."
145155
)
146156

157+
def test_pair_word_replacement_split_token_collision(
158+
self, attacked_text_pair_split_word_collision
159+
):
160+
# Regression test for https://github.com/QData/TextAttack/issues/631:
161+
# replacing the first word of the second segment ("I") used to match
162+
# inside the "<SPLIT>" join token instead of the real word, since "I"
163+
# is a character substring of "<SPLIT>".
164+
new_text = attacked_text_pair_split_word_collision.replace_word_at_index(
165+
2, "You"
166+
)
167+
assert new_text.text == "hello there\nYou am fine."
168+
147169
def test_pair_word_insertion(self, attacked_text_pair):
148170
new_text = attacked_text_pair.insert_text_after_word_index(3, "old decrepit")
149171
assert new_text.text == (
@@ -204,3 +226,20 @@ def test_hyphen_apostrophe_words(self, hyphenated_text):
204226
]
205227

206228
# TODO: test align_words_with_tokens
229+
230+
231+
def test_pos_of_word_index_after_ner_of_word_index():
232+
# Regression test for a CI failure (KeyError: 'upos', later reproduced
233+
# locally as an empty flair annotation layer): `flair_tag` cached a
234+
# single `SequenceTagger` in a module-global slot keyed by nothing, so
235+
# once anything called `ner_of_word_index` (loading the "ner" tagger)
236+
# in the same process, every later `pos_of_word_index` call silently
237+
# reused that NER tagger instead of loading "upos-fast", producing
238+
# wrong or empty POS labels. Calling NER before POS on a fresh
239+
# AttackedText reproduces the exact ordering that broke in CI.
240+
ner_text = textattack.shared.AttackedText("I am in Dallas.")
241+
ner_text.ner_of_word_index(3)
242+
243+
pos_text = textattack.shared.AttackedText("The cats were running quickly.")
244+
assert pos_text.pos_of_word_index(1) == "NOUN"
245+
assert pos_text.pos_of_word_index(2) == "VERB"

tests/test_transformations.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,62 @@ def test_chinese_word_swap_hownet():
149149
assert augmented_s or s in augmented_text_list
150150

151151

152+
def test_zip_flair_result_annotation_layer_key_independent():
153+
# zip_flair_result used to hardcode the annotation layer's key name
154+
# ("upos"), the same failure class as #727 (which hardcoded "pos" and
155+
# broke when flair started using "upos"). The actual CI failure this was
156+
# investigating turned out to be caused by a different bug (flair_tag's
157+
# single-slot tagger cache getting poisoned by a different tag_type, see
158+
# test_pos_of_word_index_after_ner_of_word_index in test_attacked_text.py
159+
# for the real regression test), but hardcoding the annotation key name
160+
# is still fragile on its own, so keep this as a defense-in-depth check
161+
# that zip_flair_result doesn't assume one specific key name.
162+
from flair.data import Sentence
163+
164+
from textattack.shared.utils import zip_flair_result
165+
166+
sentence = Sentence("cats run")
167+
for token, tag in zip(sentence.tokens, ["NOUN", "VERB"]):
168+
token.add_label("pos", tag)
169+
170+
word_list, pos_list = zip_flair_result(sentence, tag_type="upos-fast")
171+
assert word_list == ["cats", "run"]
172+
assert pos_list == ["NOUN", "VERB"]
173+
174+
175+
def test_word_swap_inflections_pos_matching():
176+
# Regression test for https://github.com/QData/TextAttack/issues/713 and
177+
# https://github.com/QData/TextAttack/issues/727: AttackedText.pos_of_word_index
178+
# returns flair's upos-fast tags (e.g. "NOUN", "VERB"), so
179+
# WordSwapInflections's POS-to-lemma mapping must have entries for those
180+
# tags, not just legacy fine-grained en-ptb tags (e.g. "NN", "VBD"), or it
181+
# silently returns zero candidates for ordinary words.
182+
import textattack
183+
from textattack.transformations.word_swaps import WordSwapInflections
184+
185+
transformation = WordSwapInflections()
186+
attacked_text = textattack.shared.AttackedText("The cats were running quickly.")
187+
188+
# Confirm the tagger is actually giving us the upos-fast tag, not a
189+
# legacy en-ptb one, so this test exercises the real mismatch and isn't
190+
# trivially passing for the wrong reason.
191+
cats_index = attacked_text.words.index("cats")
192+
cats_pos = attacked_text.pos_of_word_index(cats_index)
193+
assert cats_pos == "NOUN"
194+
# Before the fix, "NOUN" wasn't a key in the mapping (only "NN" was), so
195+
# this lookup missed and _get_replacement_words returned [] for every
196+
# ordinary noun.
197+
noun_candidates = transformation._get_replacement_words("cats", cats_pos)
198+
assert "cat" in noun_candidates
199+
200+
were_index = attacked_text.words.index("were")
201+
were_pos = attacked_text.pos_of_word_index(were_index)
202+
assert were_pos == "VERB"
203+
# Same failure mode as above, for verbs ("VERB" vs. the legacy "VBD").
204+
verb_candidates = transformation._get_replacement_words("were", were_pos)
205+
assert "was" in verb_candidates
206+
207+
152208
def test_chinese_word_swap_masked():
153209
from textattack.augmentation import Augmenter
154210
from textattack.transformations.word_swaps.chn_transformations import (

textattack/attack_recipes/bert_attack_li_2020.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@
44
55
(BERT-Attack: Adversarial Attack Against BERT Using BERT)
66
7-
.. warning::
8-
This attack is super slow
9-
(see https://github.com/QData/TextAttack/issues/586)
10-
Consider using smaller values for "max_candidates".
7+
.. note::
8+
Candidate size ``max_candidates`` defaults to 8 (down from the paper's 48)
9+
to keep runtime tractable; see https://github.com/QData/TextAttack/issues/586.
10+
Pass a larger ``max_candidates`` to ``WordSwapMaskedLM`` for closer
11+
fidelity to the original paper at the cost of much slower attacks.
1112
1213
"""
1314

@@ -39,7 +40,11 @@ class BERTAttackLi2020(AttackRecipe):
3940
def build(model_wrapper):
4041
# [from correspondence with the author]
4142
# Candidate size K is set to 48 for all data-sets.
42-
transformation = WordSwapMaskedLM(method="bert-attack", max_candidates=48)
43+
# In practice K=48 makes the number of masked-LM candidate combinations
44+
# explode for multi-subword tokens (48**n), causing multi-hour runtimes;
45+
# see https://github.com/QData/TextAttack/issues/586. K=8 keeps the attack
46+
# tractable with a small accuracy/ASR tradeoff.
47+
transformation = WordSwapMaskedLM(method="bert-attack", max_candidates=8)
4348
#
4449
# Don't modify the same word twice or stopwords.
4550
#

textattack/shared/attacked_text.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,26 @@ def generate_new_attacked_text(self, new_words: Iterable[str]) -> AttackedText:
414414
# Create the new attacked text by swapping out words from the original
415415
# text with a sequence of 0+ words in the new text.
416416
for i, (input_word, adv_word_seq) in enumerate(zip(self.words, new_words)):
417-
word_start = original_text.index(input_word)
417+
# `input_word` can be a character substring of `SPLIT_TOKEN` itself
418+
# (e.g. "I" is a substring of "<SPLIT>"), which makes a naive
419+
# `.index(input_word)` match inside the token instead of the real
420+
# word when it directly follows a split. Route around the split
421+
# token's span in that case. See
422+
# https://github.com/QData/TextAttack/issues/631
423+
if (
424+
input_word in AttackedText.SPLIT_TOKEN
425+
and AttackedText.SPLIT_TOKEN in original_text
426+
):
427+
split_start = original_text.index(AttackedText.SPLIT_TOKEN)
428+
split_end = split_start + len(AttackedText.SPLIT_TOKEN)
429+
if split_start <= original_text.index(input_word) < split_end:
430+
word_start = original_text.replace(
431+
AttackedText.SPLIT_TOKEN, ""
432+
).index(input_word) + len(AttackedText.SPLIT_TOKEN)
433+
else:
434+
word_start = original_text.index(input_word)
435+
else:
436+
word_start = original_text.index(input_word)
418437
word_end = word_start + len(input_word)
419438
perturbed_text += original_text[:word_start]
420439
original_text = original_text[word_end:]

textattack/shared/utils/strings.py

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,17 +215,21 @@ def color_text(text, color=None, method=None):
215215
return "[[" + text + "]]"
216216

217217

218-
_flair_pos_tagger = None
218+
_flair_taggers = {}
219219

220220

221221
def flair_tag(sentence, tag_type="upos-fast"):
222222
"""Tags a `Sentence` object using `flair` part-of-speech tagger."""
223-
global _flair_pos_tagger
224-
if not _flair_pos_tagger:
223+
# Cache one tagger per `tag_type`: this function is used for both POS
224+
# ("upos-fast") and NER ("ner", "flair/ner-french", ...) tagging, and a
225+
# single shared slot used to silently reuse whichever tagger loaded
226+
# first for every later call regardless of the requested `tag_type`,
227+
# corrupting results for the other tagging purpose.
228+
if tag_type not in _flair_taggers:
225229
from flair.models import SequenceTagger
226230

227-
_flair_pos_tagger = SequenceTagger.load(tag_type)
228-
_flair_pos_tagger.predict(sentence, force_token_predictions=True)
231+
_flair_taggers[tag_type] = SequenceTagger.load(tag_type)
232+
_flair_taggers[tag_type].predict(sentence, force_token_predictions=True)
229233

230234

231235
def zip_flair_result(pred, tag_type="upos-fast"):
@@ -242,7 +246,18 @@ def zip_flair_result(pred, tag_type="upos-fast"):
242246
for token in tokens:
243247
word_list.append(token.text)
244248
if "pos" in tag_type:
245-
pos_list.append(token.annotation_layers["upos"][0]._value)
249+
# The annotation layer's key (e.g. "pos", "upos") is set by
250+
# whichever flair tagger/version produced these labels, and has
251+
# changed across flair releases (see #727). Read the key that's
252+
# actually present on the token instead of hardcoding one.
253+
pos_label_type = next(iter(token.annotation_layers), None)
254+
if pos_label_type is None:
255+
raise ValueError(
256+
f"No part-of-speech label found for token {token.text!r}; "
257+
f"the flair `Sentence` may have been tagged with a "
258+
f"tagger that doesn't match tag_type={tag_type!r}."
259+
)
260+
pos_list.append(token.annotation_layers[pos_label_type][0]._value)
246261
elif tag_type == "ner":
247262
pos_list.append(token.get_label("ner"))
248263

textattack/transformations/word_swaps/word_swap_inflections.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,25 @@ class WordSwapInflections(WordSwap):
2626

2727
def __init__(self, **kwargs):
2828
super().__init__(**kwargs)
29-
# fine-grained en-ptb POS to universal POS mapping
30-
# (mapping info: https://github.com/slavpetrov/universal-pos-tags)
29+
# `AttackedText.pos_of_word_index` now returns flair's "upos-fast" tags
30+
# directly (Universal Dependencies UPOS, e.g. "NOUN", "VERB", "PROPN"),
31+
# not the old fine-grained en-ptb tags this dict's keys used to assume
32+
# (see https://github.com/QData/TextAttack/issues/713,
33+
# https://github.com/QData/TextAttack/issues/727). Without the plain
34+
# "NOUN"/"VERB"/"ADJ" entries below, this lookup always misses and the
35+
# transformation silently returns no candidates for ordinary words.
36+
# Kept the legacy en-ptb keys too in case a caller wires up a
37+
# PTB-style tagger.
38+
# (mapping info: https://universaldependencies.org/u/pos/,
39+
# https://github.com/slavpetrov/universal-pos-tags)
3140
self._enptb_to_universal = {
41+
# current flair "upos-fast" (Universal Dependencies) tags
42+
"NOUN": "NOUN",
43+
"PROPN": "NOUN",
44+
"VERB": "VERB",
45+
"AUX": "VERB",
46+
"ADJ": "ADJ",
47+
# legacy fine-grained en-ptb tags
3248
"JJRJR": "ADJ",
3349
"VBN": "VERB",
3450
"VBP": "VERB",

0 commit comments

Comments
 (0)