Conversation
Contributor
|
Documentation preview: https://vllm--39661.org.readthedocs.build/en/39661/ |
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds Gemma 4 to the supported models documentation. The review feedback identifies a formatting error where a leading pipe was omitted, suggests using the text-only architecture name for this specific table, and recommends correctly indicating LoRA support.
| | `Gemma2ForCausalLM` | Gemma 2 | `google/gemma-2-9b`, `google/gemma-2-27b`, etc. | ✅︎ | ✅︎ | | ||
| | `Gemma3ForCausalLM` | Gemma 3 | `google/gemma-3-1b-it`, etc. | ✅︎ | ✅︎ | | ||
| | `Gemma3nForCausalLM` | Gemma 3n | `google/gemma-3n-E2B-it`, `google/gemma-3n-E4B-it`, etc. | | | | ||
| `Gemma4ForConditionalGeneration` | Gemma 4 | `google/gemma-4-31B`, `google/gemma-4-26B-A4B-it`, etc. | | ✅︎ | |
Contributor
There was a problem hiding this comment.
The entry for Gemma 4 has several issues that should be addressed:
- Formatting: The line is missing the leading pipe (
|), which breaks the markdown table rendering. - Architecture Name: This table is for text-only generative models. The correct architecture name for the text-only version is
Gemma4ForCausalLM.Gemma4ForConditionalGenerationis the multimodal variant and should be listed in the Multimodal Language Models section instead. - LoRA Support: The implementation in
vllm/model_executor/models/gemma4.pyindicates that LoRA is supported (it inherits fromSupportsLoRA). The LoRA column should be marked with✅︎to reflect this capability.
Suggested change
| `Gemma4ForConditionalGeneration` | Gemma 4 | `google/gemma-4-31B`, `google/gemma-4-26B-A4B-it`, etc. | | ✅︎ | | |
| | `Gemma4ForCausalLM` | Gemma 4 | `google/gemma-4-31B`, `google/gemma-4-26B-A4B-it`, etc. | ✅︎ | ✅︎ | |
Member
|
There is already a PR #39607 |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.