-
Notifications
You must be signed in to change notification settings - Fork 426
Add SmolVLM2 support #919
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
Add SmolVLM2 support #919
Conversation
…cation - Introduced a new tokenizer configuration file for SmolVLM2-256M-Video-Instruct. - Added a function to check the availability of the SmolVLM model. - Implemented tests to ensure proper application of Liger kernels to SmolVLM2 model instances. - Added a utility function to revert Liger kernel patches for SmolVLM2.
| logger.info("Apply liger cross entropy") | ||
|
|
||
| from transformers.loss.loss_utils import nn | ||
|
|
||
| nn.functional.cross_entropy = liger_cross_entropy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unintended changes in internvl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the confusion, I'm author of PR adding InternVL3(#878) to liger-kernel and this change is intended. I checked that on the transformers version when InternVL3 and SmolVLM2 implemented they aren't using CrossEntropyLoss inside modeling_*.py instead using loss_utils. So I fixed the InternVL3 implementation togather.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you hope I'll separate this into another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's separate it into another PR and open an issue to track if there's any other cross entropy not being patched correctly!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue #920
| if vision_liger_fn: | ||
| accept_params = inspect.signature(vision_liger_fn).parameters | ||
| remain_params = set(kwargs) - (set(accept_params) & set(kwargs)) | ||
| vision_kwargs = {k: v for k, v in kwargs.items() if k not in remain_params} | ||
|
|
||
| if remain_params: | ||
| logger.warning( | ||
| f"These parameters are not supported by {vision_model_name}. Enter the remaining {list(vision_kwargs.keys())} except for {list(remain_params)}\n" | ||
| f"Parameters accepted by {vision_model_name}: {list(accept_params.keys())}" | ||
| ) | ||
| vision_kwargs["model"] = model.model.vision_model | ||
| vision_liger_fn(**vision_kwargs) | ||
| elif vision_model_name not in MODEL_TYPE_TO_APPLY_LIGER_FN: | ||
| logger.warning(f"{vision_model_name} is not supported by Liger kernel.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default SmolVLMForConditionalGeneration.vision_model is SmolVLMVisionTransformer
WARNING liger_kernel.transformers.monkey_patch:monkey_patch.py:2194 smolvlm_vision is not supported by Liger kernel.
Do you want to support it in this PR as well? (SmolVLMVisionMLP and LayerNorm)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SmolVLMVisionMLP seems not easy to patch kernel but LayerNorm seems possible, so I patched them!
Co-authored-by: Tcc0403 <[email protected]>
Co-authored-by: Tcc0403 <[email protected]>
Co-authored-by: Tcc0403 <[email protected]>
|
Ready for review again, @Tcc0403 I've checked following tests pass: There are some modifications also to InternVL, I can (1) modify this PR's title and description to include InternVL's changes or (2) seperate InternVL to separate PR. Please inform me you prefer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Seperate internvl change into another PR then we're good to merge!
|
Thank you! |
Summary
This PR adds support for SmolVLM2 to liger-kernel.
SmolVLM2 is present on transformers after 4.50.0 version: https://github.com/huggingface/transformers/releases/tag/v4.50.0
Testing Done
make testto ensure correctnessmake checkstyleto ensure code stylemake test-convergenceto ensure convergenceThere's no test failure for smolvlm2!