Skip to content
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

PeftModel is not an instance of PreTrainedModel. No liger kernels will be applied. #34016

Closed
4 tasks
ambroser53 opened this issue Oct 7, 2024 · 7 comments · Fixed by #35680
Closed
4 tasks

Comments

@ambroser53
Copy link
Contributor

System Info

transformers==4.45.1
peft==0.13.0
liger-kernel==0.3.1

So isinstance(model.base_model.model, PreTrainedModel) returns true but isinstance(model, PreTrainedModel) returns false so no liger kernels are used.

Who can help?

No response

Information

  • The official example scripts
  • My own modified scripts

Tasks

  • An officially supported task in the examples folder (such as GLUE/SQuAD, ...)
  • My own task or dataset (give details below)

Reproduction

Load any model with a PeftModel wrapper via get_peft_model and try to run with the use_liger_kernel flag in the trainer.

Expected behavior

Apply liger kernels. Be as simple as add a check for peft models that then checks model.base_model.model

@ambroser53 ambroser53 added the bug label Oct 7, 2024
@LysandreJik
Copy link
Member

cc @BenjaminBossan for the PeftModel and @ArthurZucker for the liger kernel

@ambroser53
Copy link
Contributor Author

ambroser53 commented Oct 8, 2024

I've narrowed the cause down to the changes made in #33502

@BenjaminBossan
Copy link
Member

Just to confirm, a PeftModel is not an instance of PretrainedModel, so it is expected that this check would return False. As @ambroser53 correctly identified, we can check if the model is a PEFT model and if this is true, we can check:

  1. isinstance(model.base_model, PreTrainedModel) -> apply liger to model.base_model (for prompt learning methods)
  2. isinstance(model.base_model.model, PreTrainedModel) -> apply liger to model.base_model.model (for LoRA etc.)

I don't know what exactly liger kernels do, but from the description

We have implemented Hugging Face Compatible RMSNorm, RoPE, SwiGLU, CrossEntropy, FusedLinearCrossEntropy

they should not interfere with PEFT and should be usable in conjunction. LMK what you think.

Copy link

github-actions bot commented Nov 7, 2024

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.

@ambroser53
Copy link
Contributor Author

Why has this been closed? It's not fixed and it's a very easy change. I've been having to manually do it this whole time on my local transformers version.

@BenjaminBossan
Copy link
Member

@ambroser53 The issue has been closed due to in inactivity, but as you mentioned, it's still not resolved.

I've been having to manually do it this whole time on my local transformers version.

Would you be interested in providing your fix in a PR?

Copy link

github-actions bot commented Jan 7, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants