fix(gemma3): don't cast attention mask to packed uint32 weight dtype — padding leaks into embeddings of quantized checkpoints - #73
Open
alekseysotnikov wants to merge 1 commit into
Conversation
…ed weight dtype Quantized checkpoints pack embed_tokens.weight as uint32. Casting the 0.0/-inf additive attention mask to that dtype silently destroys the -inf padding entries, so pad tokens leak into attention on every padded batch (the documented batch-processing path). On mlx-community/embeddinggemma-300m-8bit, an 11-char sentence embedded alone vs batched with 127 max-length sentences: cos 0.884 before, 0.999997 after the fix. On a real 395-sentence transcript, mean cosine vs zero-padding ground truth was 0.928 (385/395 sentences < 0.99) with the current code.
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.
Problem
Model.__call__inmlx_embeddings/models/gemma3_text.pycasts the additive attention mask toself.model.embed_tokens.weight.dtype:In quantized checkpoints (e.g.
mlx-community/embeddinggemma-300m-8bit)embed_tokens.weight.dtypeis packed uint32. Casting the0.0 / -infadditive mask to uint32 silently destroys the-infentries, so pad tokens leak into attention whenever padded batches are used — which is the documented batch-processing path (batch_encode_plus(padding=True)+attention_mask).This affects any quantized gemma3 embedding model; fp16-weight checkpoints work only by accident because their weight dtype happens to be floating.
Minimal repro
EmbeddingGemma-300m-8bit, one 11-char sentence:
mx.float16insteadQuantifying over a full real-world transcript (395 sentences, natural-order batching): mean cosine vs zero-padding ground truth is 0.928 (385/395 sentences below 0.99) with current code — i.e. nearly every embedding is distorted by its batch neighbors' padding.
Fix
Cast the mask to a floating dtype instead of the (possibly packed-integer) weight dtype: pick the first floating parameter dtype (respects bf16 checkpoints), falling back to
mx.float16. One-line behavioral change, no API impact.Verified: parity between per-sentence, natural-order and length-sorted batching goes to cos ≈ 1.0 on both reproductions above.
Fixes #72