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.

Sign up for GitHub

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

Fix list index out of range when padding nested empty lists #13876

Merged
merged 4 commits into from
Nov 10, 2021
Merged

Fix list index out of range when padding nested empty lists #13876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
632f21f
Fix index out of range when padding
qqaatw Oct 5, 2021
c3b027b
Apply suggestions from code review
qqaatw Nov 10, 2021
3d9456b
Style
qqaatw Nov 10, 2021
fdb6b7f
Merge branch 'master' into fix_pad
qqaatw Nov 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/transformers/tokenization_utils_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2733,11 +2733,10 @@ def pad(
first_element = required_input[0]
if isinstance(first_element, (list, tuple)):
# first_element might be an empty list/tuple in some edge cases so we grab the first non empty element.
index = 0
while len(required_input[index]) == 0:
index += 1
if index < len(required_input):
first_element = required_input[index][0]
for item in required_input:
if len(item) != 0:
first_element = item[0]
break
# At this state, if `first_element` is still a list/tuple, it's an empty one so there is nothing to do.
if not isinstance(first_element, (int, list, tuple)):
if is_tf_available() and _is_tensorflow(first_element):
Expand Down
22 changes: 13 additions & 9 deletions tests/test_tokenization_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,7 @@ def test_right_and_left_padding(self):
assert encoded_sequence == padded_sequence_left

def test_padding_to_max_length(self):
"""We keep this test for backward compatibility but it should be remove when `pad_to_max_length` will e deprecated"""
"""We keep this test for backward compatibility but it should be remove when `pad_to_max_length` is deprecated."""
tokenizers = self.get_tokenizers(do_lower_case=False)
for tokenizer in tokenizers:
with self.subTest(f"{tokenizer.__class__.__name__}"):
Expand Down Expand Up @@ -2997,8 +2997,8 @@ def test_padding(self, max_length=50):
input_r = tokenizer_r.encode_plus("This is a input 1")
input_r = tokenizer_r.pad(input_r)

input_p = tokenizer_r.encode_plus("This is a input 1")
input_p = tokenizer_r.pad(input_p)
input_p = tokenizer_p.encode_plus("This is a input 1")
input_p = tokenizer_p.pad(input_p)
Comment on lines +3000 to +3001
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is an overlooking when copying test cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh! Great catch! Thanks! 👍


self.assert_padded_input_match(
input_r["input_ids"], input_p["input_ids"], len(input_r["input_ids"]), pad_token_id
Expand All @@ -3008,8 +3008,8 @@ def test_padding(self, max_length=50):
input_r = tokenizer_r.encode_plus("This is a input 1")
input_r = tokenizer_r.pad(input_r, max_length=max_length, padding="max_length")

input_p = tokenizer_r.encode_plus("This is a input 1")
input_p = tokenizer_r.pad(input_p, max_length=max_length, padding="max_length")
input_p = tokenizer_p.encode_plus("This is a input 1")
input_p = tokenizer_p.pad(input_p, max_length=max_length, padding="max_length")

self.assert_padded_input_match(input_r["input_ids"], input_p["input_ids"], max_length, pad_token_id)

Expand All @@ -3019,10 +3019,10 @@ def test_padding(self, max_length=50):
)
input_r = tokenizer_r.pad(input_r)

input_p = tokenizer_r.batch_encode_plus(
input_p = tokenizer_p.batch_encode_plus(
["This is a input 1", "This is a much longer input whilch should be padded"]
)
input_p = tokenizer_r.pad(input_p)
input_p = tokenizer_p.pad(input_p)

self.assert_batch_padded_input_match(input_r, input_p, len(input_r["input_ids"][0]), pad_token_id)

Expand All @@ -3032,11 +3032,15 @@ def test_padding(self, max_length=50):
)
input_r = tokenizer_r.pad(input_r, max_length=max_length, padding="max_length")

input_p = tokenizer_r.batch_encode_plus(
input_p = tokenizer_p.batch_encode_plus(
["This is a input 1", "This is a much longer input whilch should be padded"]
)
input_p = tokenizer_r.pad(input_p, max_length=max_length, padding="max_length")
input_p = tokenizer_p.pad(input_p, max_length=max_length, padding="max_length")
self.assert_batch_padded_input_match(input_r, input_p, max_length, pad_token_id)

# Test padding nested empty lists (in some use-cases, there is no any token id in the `input_ids` list).
input_r = tokenizer_r.pad({"input_ids": [[], []]}, max_length=max_length, padding="max_length")
input_p = tokenizer_p.pad({"input_ids": [[], []]}, max_length=max_length, padding="max_length")
self.assert_batch_padded_input_match(input_r, input_p, max_length, pad_token_id)

def test_padding_different_model_input_name(self):
Expand Down