added warning to Trainer when label_names is not specified for PeftModel

[MilkClouds]

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.

[amyeroberts]

Thanks for adding this check - LGTM but let's get confirmation from @muellerzr

[amyeroberts]

I don't think this check is robust as the model could be e.g. PeftForCausalLM. Within this module we have _is_peft_model which I think we should use.

[MilkClouds]

ok, very good suggestion!

[muellerzr]

Thanks, agree it makes sense but cc @BenjaminBossan here since it's peft :)

[HuggingFaceDocBuilderDev]

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.

[amyeroberts]

Thanks for iterating on this!

If @BenjaminBossan is happy with it I think we can merge :)

[BenjaminBossan]

Thanks for the PR.

Not sure if I fully understand the issue, does it have to do with some function signature inspection that fails on the wrapped model because it uses *args, **kwargs? Could you maybe provide a small example where this comes up?

[MilkClouds]

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.

[BenjaminBossan]

Thanks for clarifying, that makes sense. I had already shared this concern internally in the past (but got no response 😢):

Users are reporting issues with Trainer not showing evals when using PEFT models (e.g. huggingface/peft#1881). The core of the problem appear to functions like

• can_return_loss
• find_labels

that inspect the forward signature of the model class. As PEFT wraps the transformers model and uses mostly **kwargs in the signature, this check returns False even if the underlying model would return True.
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 a base_model attribute, 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.

[github-actions]

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.

[MilkClouds]

Does it need any other consideration, @amyeroberts ?

[ArthurZucker]

I think we were waiting for the suggestion from @BenjaminBossan to be merged 🤗

[MilkClouds]

Ah, I see. I missed the suggestion. I accepted the suggestion!

[ArthurZucker]

Now let's make sure make fixup and the rest of the CIs related are green!

[MilkClouds]

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.

[ArthurZucker]

Failing test is unrelated don't worry!