Generation: get special tokens from model config #30899
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
There was a problem hiding this comment.
I see what went wrong in my previous PR 😭 Thank you for taking this issue @zucchini-nlp!
For full BC, we need further changes. The removed _get_decoder_start_token_id function used bos_token_id as a fallback, and bos_token_id could be loaded from the config (like decoder_start_token_id). Furthermore, this logic only was used when self.config.is_encoder_decoder is True.
So the needed changes for BC are:
- first check if
self.config.is_encoder_decoder is True. - If 1. is true, get BOTH
decoder_start_token_idandbos_token_idwith fallback toself.generation_config - if 1. is false, don't use a fallback to
self.generation_config - The line
decoder_start_token_id = decoder_start_token_id if decoder_start_token_id is not None else bos_token_idcan stay as it is :)
@ArthurZucker see my comment above :D |
|
@ArthurZucker @gante done, I brought back the |
| elif bos_token_id is not None: | ||
| return bos_token_id | ||
| else: | ||
| return |
There was a problem hiding this comment.
| return | |
| return None |
explicit but I guess it's the same
* fix * let's do this way? * codestyle * update * add tests
* fix * let's do this way? * codestyle * update * add tests
What does this PR do?
Fixed #30892 . The error, described in the linked issue, was caused by passing a
GenerationConfigtype instead of a kwarg. If a user passesGenerationConfigwe do not update it with model's generation config, thus cannot get special tokens used by the model. Two options were:GenerationConfigby that of models in the_prepare_generation_configI think the first option can mess-up with what the user wants when generating, for ex by adding a
do_sample=Truefrom model's generation config. So I went for option-2