transformers
transformers copied to clipboard
added warning to Trainer when label_names is not specified for PeftModel
What does this PR do?
in Trainer, it automatically finds label_names with input arguments name of model.
But with model which is PeftModel, the input arguments are all hidden and we can't find the input argument's name automatically.
So, for the case then the user didn't specified label_names in TrainerArguments, I made warning message pop up.
p.s. it makes the change when I add call trainer.predict; if label_names it not set, there's no way I can get get label_ids from trainer.predict outcomes.
Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [ ] Did you read the contributor guideline, Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the documentation guidelines, and here are tips on formatting docstrings.
- [ ] Did you write any new necessary tests?
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag members/contributors who may be interested in your PR.
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.
Basically inside Trainer's init function, it automatically finds whether 'labels' argument exist in model class's forward function.
e.g. LlamaForCausalLM: have labels field in forward, so 'labels' is detected e.g. PeftModel-wrapped LlamaForCausalLM: since PeftModel only have *args, **kwargs in forward's argument, it can't detect 'labels' of LlamaForCausalLM.
Thanks for clarifying, that makes sense. I had already shared this concern internally in the past (but got no response :cry:):
Users are reporting issues with Trainer not showing evals when using PEFT models (e.g. https://github.com/huggingface/peft/issues/1881). The core of the problem appear to functions like
that inspect the forward signature of the model class. As PEFT wraps the transformers model and uses mostly
**kwargsin the signature, this check returnsFalseeven if the underlying model would returnTrue. I'm not sure how to resolve this. On the PEFT side, I could just add the arguments to the signature, but this can lead to false positives, as the base model may or may not actually support this. Therefore, it would be better if on the transformers side there would be a check if the model is a PEFT model, or if it has abase_modelattribute, and then check the signature of the base model instead. This of course won't work if the model class is passed, it would require the model instance, so this would be a somewhat bigger change.
So I guess this PR helps a little bit but the fundamental problem still remains. Thus from my side, it's okay to merge with my suggestion applied.
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.
Does it need any other consideration, @amyeroberts ?
I think we were waiting for the suggestion from @BenjaminBossan to be merged 🤗
Ah, I see. I missed the suggestion. I accepted the suggestion!
Now let's make sure make fixup and the rest of the CIs related are green!
Sorry for the late reply, I've forgotten and thought this PR was merged.
I ran make fixup and fixed some, but some of CI/CD fail until now.
Failing test is unrelated don't worry!
Basically inside Trainer's init function, it automatically finds whether 'labels' argument exist in model class's forward function.
e.g. LlamaForCausalLM: have labels field in forward, so 'labels' is detected e.g. PeftModel-wrapped LlamaForCausalLM: since PeftModel only have *args, **kwargs in forward's argument, it can't detect 'labels' of LlamaForCausalLM.
Yes, I am using LlamaForCausalLM, and after I give my lora_config and use "model = get_peft_model(model, lora_config)", the trainer will still remind me "No label_names provided for model class PeftModelForCausalLM. Since PeftModel hides base models input arguments". Could you tell me how you solve this problem? Thank you very much.