Repository navigation
EmbeddingGemma-2 support - #247
danielhanchen wants to merge 2 commits into
Conversation
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73c2f733e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| class EmbeddingGemma2VisionAudioModel(Gemma4VisionAudioModel): | ||
| def modify_tensors(self, data_torch: Tensor, name: str, bid: int | None) -> Iterable[tuple[str, Tensor]]: | ||
| # same towers as Gemma4, but the tensor names have no "model." prefix | ||
| yield from super().modify_tensors(data_torch, "model." + name, bid) |
There was a problem hiding this comment.
When --mmproj processes an EmbeddingGemma2 checkpoint, the inherited MmprojModel.filter_tensors() rejects only language_model.*, but this architecture's language weights are prefixless (layers.*, ple.*, and embedding_projection.*). They therefore reach this method, receive an invalid model. prefix, and are passed through the vision mapping path, where map_tensor_name() raises ValueError: Can not map tensor ...; consequently the newly added image/audio projector cannot be converted from the normal full checkpoint. Add a model-specific filter that retains only the vision and audio tower tensors before applying the prefix.
Useful? React with 👍 / 👎.
| ggml_tensor * llama_model_gemma_embedding2::graph::build_inp_per_layer(ggml_tensor * inpL) { | ||
| const int64_t n_embd_per_layer = hparams.n_embd_per_layer; | ||
|
|
||
| ggml_tensor * cur = ggml_mul_mat(ctx0, model.per_layer_model_proj, inpL); // [n_embd_per_layer * n_layer, n_tokens] |
There was a problem hiding this comment.
When a LoRA includes weights for per_layer_model_proj, this raw ggml_mul_mat bypasses the adapter lookup performed by build_lora_mm, even though the other linear projections in this new graph use that helper. The adapter can therefore load without an error while this projection continues using only the base weight, producing incorrect per-layer inputs and embeddings; route this multiplication through build_lora_mm as well.
AGENTS.md reference: AGENTS.md:L83-L83
Useful? React with 👍 / 👎.
|
Upstream ggml-org#30054 landed EmbeddingGemma-2 support, and tags from b11452 on carry all of it, so this pin is no longer needed. |
…at merge past b11453, drop unslothai#247, pin unslothai#251 Upstream K2 Horizon (ggml-org#29535), the glm5-next gather removal (ggml-org#30042) and the GLM5-Next MTP graph (ggml-org#29928) broke the Inkling, unslothai#243 and unslothai#241 pins. Each branch now carries a merge of upstream master. unslothai#247 is in every tag from b11452 on. unslothai#251 adds --moe-cache-mib auto on top of ggml-org#29887.
unsloth: repin Inkling, unslothai#241 and unslothai#243 past b11453, drop unslothai#247, pin --moe-cache-mib auto (unslothai#251)
2 participants
Summary
EmbeddingGemma 2 support (google/embeddinggemma-2), by @ngxson, for the nightly pin set.
Two commits by @ngxson, cherry-picked unchanged onto
b11436(the current nightly base):model: add embeddinggemma2: thegemma-embedding2architecture, its graph and the converter.add mtmd support: images and audio through the existing Gemma 4 vision and audio projectors.Together they are +324 / -2 across 10 files, the same lines as the upstream change. This replaces #246, which put source into fork master.
Testing
Local mirror of
unsloth-pin-preflight.ymlonb11436:merge_checks.pyis clean andpin_contract.pyreports all pins intact.llama-embeddingon the BF16 GGUF built from the merged tree matches a build of the upstream change.