Deprecate prepare_seq2seq_batch #10287
Merged
Conversation
This file contains 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
sgugger
commented
Feb 19, 2021
Comment on lines
+88
to
+89
| def __init__(self, *args, tokenizer_file=None, src_lang=None, tgt_lang=None, **kwargs): | ||
| super().__init__(*args, tokenizer_file=tokenizer_file, src_lang=src_lang, tgt_lang=tgt_lang, **kwargs) |
There was a problem hiding this comment.
Add the ability to set src_lang and tgt_lang at init.
Comment on lines
+111
to
+119
| @property | ||
| def src_lang(self) -> str: | ||
| return self._src_lang | ||
|
|
||
| @src_lang.setter | ||
| def src_lang(self, new_src_lang: str) -> None: | ||
| self._src_lang = new_src_lang | ||
| self.set_src_lang_special_tokens(self._src_lang) | ||
|
|
There was a problem hiding this comment.
Add the proper setter for src_lang.
Comment on lines
-134
to
-138
| # test None max_target_length | ||
| batch = tokenizer.prepare_seq2seq_batch( | ||
| src_text, tgt_texts=tgt_text, max_length=32, padding="max_length", return_tensors="pt" | ||
| ) | ||
| self.assertEqual(32, batch["labels"].shape[1]) |
There was a problem hiding this comment.
This second part is irrelevant to test now.
LysandreJik
approved these changes
Feb 19, 2021
docs/source/model_doc/pegasus.rst
Outdated
| @@ -85,10 +85,10 @@ Usage Example | |||
| ] | |||
|
|
|||
| model_name = 'google/pegasus-xsum' | |||
| torch_device = 'cuda' if torch.cuda.is_available() else 'cpu' | |||
| device = 'cuda' if torch.cuda.is_available() else 'cpu' | |||
patil-suraj
approved these changes
Feb 22, 2021
tests/test_tokenization_pegasus.py
Outdated
| assert batch.labels.shape == (2, 5) | ||
| assert len(batch) == 3 # input_ids, attention_mask, labels. Other things make by BartModel | ||
| assert targets["input_ids"].shape == (2, 5) | ||
| assert len(batch) == 2 # input_ids, attention_mask. Other things make by BartModel |
There was a problem hiding this comment.
(nit) Think we can remove the Other things make by BartModel
Comment on lines
40
to
44
| >>> inputs = tokenizer([article], return_tensors="pt") | ||
| >>> with tokenizer.as_target_tokenizer(): | ||
| ... labels = tokenizer([summary], return_tensors="pt") | ||
|
|
||
| >>> outputs = model(input_ids=inputs["input_ids"], decoder_input_ids=labels["input_ids"]) |
There was a problem hiding this comment.
(nit) I think it would be better to pass article and summary as either string or list in all examples to ensure consistency in docs. Some examples are using lists and some are directly passing the single string.
patrickvonplaten
approved these changes
Feb 22, 2021
Co-authored-by: Lysandre Debut <lysandre@huggingface.co> Co-authored-by: Suraj Patil <surajp815@gmail.com>
|
Hi all! Sorry, but this seems to be cleaner: (Some feature request: #14255) encoded_train_dataset = train_dataset.map(
lambda batch: tokenizer.prepare_seq2seq_batch(
batch['text'], batch['summary'], padding='max_length', truncation=True, max_length=256, max_target_length=64
),
batched=True,
remove_columns=train_dataset.column_names,
) |
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.
What does this PR do?
This PR officially deprecates
prepare_seq2seq_batchto prepare its removal in Transformers v5. As discussed before, the proper way to prepare data for sequence-to-sequence tasks is to:as_target_tokenizerWhen only dealing with input texts without targets, just using the tokenizer call works perfectly well.
For
mBARTandmBART50tokenizers the source and target language can be specified at init or changed at any time by setting the attributes.src_langand.tgt_lang.Here is a full example showing how to port old code using
prepare_seq2seq_batchto the new way in the case of an mBART tokenizer (remove the mentiones ofsrc_langandtgt_langfor other tokenizers:becomes
The languages can be changed at any time with
This PR fixes a few things in
MBartTokenizerandMBartTokenizerFastfor the new API to work completely and removes all mentions ofprepare_seq2seq_batchfrom the documentation and tests (except the test of that method in the common tests). It was already not used anymore in the seq2seq examplerun_seq2seq.