Skip to content

Conversation

@Tcc0403
Copy link
Collaborator

@Tcc0403 Tcc0403 commented Dec 3, 2025

Summary

Fix #943

Testing Done

  • Hardware Type:
  • run make test to ensure correctness
  • run make checkstyle to ensure code style
  • run make test-convergence to ensure convergence

Signed-off-by: Tcc0403 <[email protected]>
@Tcc0403
Copy link
Collaborator Author

Tcc0403 commented Dec 3, 2025

@sfc-gh-sbekman quick question! Do you think we should patch liger kernel on model instances created by from_pretrained and from_config? or global patching as we currently do is fine?

Right now, AutoLigerKernelForCausalLM.from_pretrained and AutoLigerKernelForCausalLM.from_config apply liger kernel patch on global symbols instead of model instances, which might cause side effect on subsequent model initializations. For instance, you can't initialize the same model without liger patch after the first AutoLigerKernelForCausalLM.from_pretrained call.

@Tcc0403 Tcc0403 changed the title Add from_config auto factory method Add AutoLigerKernelForCausalLM.from_config Dec 3, 2025
@sfc-gh-sbekman
Copy link

sfc-gh-sbekman commented Dec 3, 2025

I don't have a sufficient bird's eye view that you have to tell, since your current approach always worked for my use cases.

I'd say if you want to make it more flexible, continue doing what you have been doing, but provide an additional flag like liger_kernel_apply_scope=["class", "instance"] with "class" being the default (current behavior).

@Tcc0403
Copy link
Collaborator Author

Tcc0403 commented Dec 3, 2025

Thank you for your feedback! I was wondering whether the approach brings inconvenience. If it's no concern for most people, I'll leave it what it is.

@sfc-gh-sbekman
Copy link

As I suggested, if you have users complain then it'd help, otherwise nothing needs to be done (and can easily be done down the road)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AutoLigerKernelForCausalLM.from_config support

3 participants