Include padding mask in generation #2096
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
rshraga
reviewed
Mar 7, 2023
| @@ -48,7 +48,7 @@ def _prepare_decoder_ids_for_generation( | |||
| return torch.ones((batch_size, 1), dtype=torch.long, device=device) * pad_idx | |||
|
|
|||
| def greedy_search( | |||
| self, input_ids: torch.Tensor, max_length: int, eos_idx: int, pad_idx: Optional[int] = None, **model_kwargs | |||
| self, input_ids: torch.Tensor, max_length: int, eos_idx: int, pad_idx: int, **model_kwargs | |||
There was a problem hiding this comment.
Does changing pas_idx from Optional to required break any call sites?
There was a problem hiding this comment.
Nope. Only being called from the entry point method atm.
|
|
||
| # Append the next tokens to the previous tokens | ||
| input_ids = torch.cat([input_ids, next_tokens], dim=-1) | ||
| input_ids = torch.cat([input_ids, next_tokens[:, None]], dim=-1) |
There was a problem hiding this comment.
what does the [:, None] do here?
There was a problem hiding this comment.
Same thing as unsqueezing the last dim
Nayef211
reviewed
Mar 7, 2023
| tokens_for_single_example = generation_model.generate(inputs, num_beams=1, max_length=30) | ||
| generated_text_for_single_example = self.transform.decode(tokens_for_single_example.tolist()) | ||
|
|
||
| self.assertEqual(generated_text[0], generated_text_for_single_example[-1]) |
There was a problem hiding this comment.
Why do we do generated_text_for_single_example[-1] instead of generated_text_for_single_example[0]?
There was a problem hiding this comment.
Was originally going to pass multiple through the second pass, but did not. Both get the same result though. -1 == 0
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.
Bug
Expect batched input to match single input e.g.
Before this would not create the same output1. The issue was that the src_key_padding_mask was not being propagated forward.
Fix
Create padding mask and add it to
model_kwargsand pass it to the forward function.