-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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.
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
Make it possible to customize generation config for Trainer's training loop evaluation #24427
Comments
You can customize the model |
Maybe it's because I was setting the Generation Config in the model without putting _from_model_config = False and it was triggering the following, which resets the config: if generation_config is None:
# legacy: users may modify the model configuration to control generation -- update the generation config
# model attribute accordingly, if it was created from the model config
if self.generation_config._from_model_config:
new_generation_config = GenerationConfig.from_model_config(self.config)
if new_generation_config != self.generation_config:
warnings.warn(
"You have modified the pretrained model configuration to control generation. This is a"
" deprecated strategy to control generation and will be removed soon, in a future version."
" Please use a generation configuration file (see"
" https://huggingface.co/docs/transformers/main_classes/text_generation)"
)
self.generation_config = new_generation_config
generation_config = self.generation_config |
Yeah, that was it. Though it is a bit confusing because that message says it's a deprecated strategy but doesn't say it will revert the configs. So, maybe docs should be improved to say when Generation Config is used where, with what priority between the default one, the model one, the gen_kwargs, etc... For example, in Seq2SeqTrainer.evaluate and predict there is this code which doesn't take into consideration if there is a generation config set in the model. gen_kwargs = gen_kwargs.copy()
if gen_kwargs.get("max_length") is None and gen_kwargs.get("max_new_tokens") is None:
gen_kwargs["max_length"] = self.args.generation_max_length
gen_kwargs["num_beams"] = (
gen_kwargs["num_beams"] if gen_kwargs.get("num_beams") is not None else self.args.generation_num_beams
) Furthermore, in Seq2SeqTrainer.prediction_step there is this code that, again, doesn't take into account the model generation config and goes for the gen_kwargs (which aren't passed in the training loop). gen_kwargs = self._gen_kwargs.copy()
if gen_kwargs.get("max_length") is None and gen_kwargs.get("max_new_tokens") is None:
gen_kwargs["max_length"] = self.model.config.max_length
gen_kwargs["num_beams"] = (
gen_kwargs["num_beams"] if gen_kwargs.get("num_beams") is not None else self.model.config.num_beams
)
default_synced_gpus = True if is_deepspeed_zero3_enabled() else False
gen_kwargs["synced_gpus"] = (
gen_kwargs["synced_gpus"] if gen_kwargs.get("synced_gpus") is not None else default_synced_gpus
) If you do have a Generation Config set in the model or passed as generation_config parameter in evaluate/predict, where you set So it seems if I set a GenerationConfig in the model, with max_new_tokens, then I will always get this warning because the training loop doesn't pass anything directly to evaluate/predict. Let me know if I should close this. |
Also, the shape[1] of outputs.predictions coming out of Seq2SeqTrainer.predict is 20 and it does not respect max_new_tokens passed in the generation config. |
Hi @antonioalegria 👋 We do support parameterizing Most of the issues you described are intentional temporary workarounds -- when a new feature is introduced, we have to ensure our library goes through a deprecation cycle, in which the old way of doing things take precedence. That's why you see strange patterns like the use of That being said, let's start with the basics (which you haven't done 😉 ): what version of transformers are you using, and how can I reproduce your issue? |
Hi, I have removed the I still get those "Both max_new_tokens and max_length seem to have been set." warnings though. transformers: 4.30.2 To reproduce use this code you just need to call You will then see those warnings, which shouldn't happen. Let me know if you'd like me to provide a running script. |
So what is the correct way to parametrize the generation (e.g. to use contrastive search) during the model training? The documentation misses this point. |
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. Please note that issues that do not follow the contributing guidelines are likely to be ignored. |
@gante @sgugger sorry to bother you, but it seems like the issue with GenerationConfig has not been resolved. Please, take a look at this code:
I check the config before starting the training, and everything looks fine:
Then during the evaluation phase of the training I print the generated sentences and notice that the model doesn't follow my generation config. The forced start and end tokens are ignored and the number of generated tokens is 20 for any sample. I then stop the training and run the previous command again, and to my surprise see the following:
Why has the config been reset and how can I avoid it? It seems like a bug to me. @antonioalegria FYI |
Upd: the source of the problem was my I managed to achieve the desired behavior by modifying my model_init() function like that:
However, now at each evaluation I get this annoying warning:
At this point I'm not sure where does the
It seems like it would be more convenient if the explicitly passed GenerationConfig would be automatically applied to the model returned by the |
Hey @nick-maykr 👋 Thank you for raising the issue! I've just opened a PR that tackles this issue (fine-tuning from older models often ignoring As for the Let me know if you come across further issues. As I've written above, we have changed how we control generation late 2022, and maintaining retrocompatibility on top of the new features is challenging :) |
Feature request
When using
predict_with_generate
and we want to compute generation-based metrics during the eval happening during training, it would be good if the model's generation config is used and/or if we can pass the indented generation config into the train method so that it can be passed to evaluate.As it is, the generation done is using the default parameters only.
Motivation
The current way GenerationConfigs are used is pretty inconsistent and muddled IMO. You can set it at the model level but it's only used sometimes. You can pass it directly to evaluate, predict or generate but it's not clear if you should pass it as kwargs or as a full GenerationConfig.
Would be great to clean this up so that it's super clear on how to use it and have a very consistent way to use it, as in python. My suggestion would be to set it at the Trainer level and be able to override it in the evaluate, predict, generate methods with a simple generation_config: GenerationConfig parameter.
Your contribution
Happy to discuss different possibilities and see where I could help.
The text was updated successfully, but these errors were encountered: