Generate: add missing **model_kwargs in sample tests
#18696
Conversation
gante
changed the title
**model_kwargs in sample tests
| @@ -327,6 +327,7 @@ def _sample_generate( | |||
| remove_invalid_values=True, | |||
| **logits_warper_kwargs, | |||
| **process_kwargs, | |||
| **model_kwargs, | |||
There was a problem hiding this comment.
The dict only carries the attention mask :)
|
The documentation is not available anymore as the PR was closed or merged. |
There was a problem hiding this comment.
Thank you @gante . We can finally sleep better without seeing this pop again :-)
It is somehow mysterious that without passing attn mask, we have more frequent test failure.
It might be a good idea to keep this on a TODO list somewhere, either a comment above the changed line, or on Notion.
|
@ydshieh when the attention mask is not passed, it is inferred from the input (here). Depending on the pad and eos tokens, the mask inferred from the random input tokens may have issues. This automatic attention mask is a big source of issues in general, especially for the tests :( Passing the mask explicitly is always preferred! |
What does this PR do?
One call of the
sample-related tests was missing**model_kwargs, which MAY explain the random failures we were seeing.I've run all
test_sample_generate_dict_outputtests 100x with no failures. Before this change, it was failing once every ~10 calls ofpy.test tests/ -k test_sample_generate_dict_output.