Skip to content

fix(vision): make Gemma 3n VLM per_layer_inputs sequence-aware - #86

Merged
inureyes merged 1 commit into
mainfrom
fix/issue-85-gemma3n-per-layer-inputs
May 25, 2026
Merged

inureyes merged 1 commit into
mainfrom
fix/issue-85-gemma3n-per-layer-inputs

Conversation

@inureyes

Copy link
Copy Markdown
Member

Summary

Gemma 3n VLM stashed its projected per_layer_inputs tensor in a single RefCell<Option<…>> cell that get_input_embeddings wrote and forward_with_embeddings drained via take().unwrap(). The batch scheduler runs embedding-prep at enqueue time but prefill asynchronously, so a burst of concurrent Gemma 3n VLM requests could overwrite the cell before the first prefill consumed it — the first prefill then saw the wrong row's tensor, or panicked on None when the timing flipped. This affects only the batched mlxcel-server path; single-row CLI / bench paths are unaffected.

Fix

Replace the cell with Gemma3nPerLayerInputsState (a per-SequenceId map plus a fallback slot), mirroring the existing Gemma4PerLayerInputsState:

  • Override forward_with_embeddings_and_sequence_id on Gemma3nVLModel so prefill resolves from the seq-keyed map first and falls back to the legacy slot for CLI / bench / single-row callers; the legacy forward_with_embeddings routes through it with seq_id = None.
  • Wire release_sequence_state_by_id to drain the map alongside the scheduler's per-sequence cache cleanup.
  • Add the LoadedModel capability methods (bind / take / install_gemma3n_per_layer_inputs_*) and hook them into drain_incoming_requests and the preemption round trip, next to the Gemma 4 calls.

Testing

  • make verify-clippy (--all-targets --features metal,accelerate -- -D warnings) — clean
  • make verify-fmt — clean
  • New Gemma3nPerLayerInputsState unit tests (burst-enqueue race, fallback round trip, release semantics, preemption round trip) — 8/8 pass (--ignored, serial MLX)

Closes #85

Gemma 3n VLM stashed its projected per_layer_inputs tensor in a single RefCell<Option<...>> cell that get_input_embeddings wrote and forward_with_embeddings drained via take().unwrap(). The batch scheduler runs embedding-prep at enqueue time but prefill asynchronously, so a burst of concurrent Gemma 3n VLM requests could overwrite the cell before the first prefill consumed it — the first prefill then saw the wrong row's tensor, or panicked on None when the timing flipped.

Replace the cell with Gemma3nPerLayerInputsState, a per-SequenceId map plus a fallback slot, mirroring the existing Gemma4PerLayerInputsState container. Override forward_with_embeddings_and_sequence_id on Gemma3nVLModel so prefill resolves the tensor from the seq-keyed map first and only falls back to the legacy slot for CLI / bench / single-row callers; route the legacy forward_with_embeddings through it with seq_id=None. Wire release_sequence_state_by_id to drain the map alongside the scheduler's per-sequence cache cleanup, add the matching LoadedModel capability methods, and hook them into drain_incoming_requests and the preemption round trip next to the Gemma 4 calls.

Unit tests cover the burst-enqueue race, the fallback round trip, release semantics, and the preemption round trip; they are marked #[ignore] for serial MLX execution like the Gemma 4 helper tests.
@inureyes inureyes added type:bug Bug fixes, error corrections, or issue resolutions priority:high High priority area:models Model architectures, weights, loading, metadata area:inference Generation, sampling, decoding (incl. speculative, DRY) labels May 25, 2026
@inureyes
inureyes merged commit 97042eb into main May 25, 2026
4 checks passed
@inureyes
inureyes deleted the fix/issue-85-gemma3n-per-layer-inputs branch May 25, 2026 04:35
inureyes added a commit that referenced this pull request May 26, 2026
Bump version to 0.0.31. Highlights: MiniCPM-V 4.6 and RT-DETRv2 model support plus an Anthropic-style /v1/messages API endpoint (#82, #80, #74); fixes for missing or null chat-message content, Gemma 3n VLM per_layer_inputs, Qwen3.5 MTP verify attention, and batched quantized KV cache mask offsets (#91, #86, #78, #76); and a pinned 1.93.1 Rust toolchain (#87, #90).
inureyes added a commit that referenced this pull request May 26, 2026
The v0.0.31 changelog and debian entries described the Gemma 3n VLM per_layer_inputs fix as keeping per-layer input shapes correct, which undersells it: the real bug was a batch-scheduler concurrency race where a burst of concurrent VLM requests overwrote a single shared cell before prefill consumed it, reading the wrong sequence's tensor. Reword both entries to match the GitHub release note. The v0.0.31 tag is unchanged, so this lands on main for the next release.
@inureyes inureyes added this to the 0.1 milestone Jun 21, 2026
@inureyes inureyes self-assigned this Aug 31, 2026
@inureyes inureyes added the status:done Completed label Sep 6, 2026
be-free-snowboy pushed a commit to freestylejs/snowbrain-mlxcel that referenced this pull request Oct 27, 2026
…p#86)

Gemma 3n VLM stashed its projected per_layer_inputs tensor in a single RefCell<Option<...>> cell that get_input_embeddings wrote and forward_with_embeddings drained via take().unwrap(). The batch scheduler runs embedding-prep at enqueue time but prefill asynchronously, so a burst of concurrent Gemma 3n VLM requests could overwrite the cell before the first prefill consumed it — the first prefill then saw the wrong row's tensor, or panicked on None when the timing flipped.

Replace the cell with Gemma3nPerLayerInputsState, a per-SequenceId map plus a fallback slot, mirroring the existing Gemma4PerLayerInputsState container. Override forward_with_embeddings_and_sequence_id on Gemma3nVLModel so prefill resolves the tensor from the seq-keyed map first and only falls back to the legacy slot for CLI / bench / single-row callers; route the legacy forward_with_embeddings through it with seq_id=None. Wire release_sequence_state_by_id to drain the map alongside the scheduler's per-sequence cache cleanup, add the matching LoadedModel capability methods, and hook them into drain_incoming_requests and the preemption round trip next to the Gemma 4 calls.

Unit tests cover the burst-enqueue race, the fallback round trip, release semantics, and the preemption round trip; they are marked #[ignore] for serial MLX execution like the Gemma 4 helper tests.
be-free-snowboy pushed a commit to freestylejs/snowbrain-mlxcel that referenced this pull request Oct 27, 2026
Bump version to 0.0.31. Highlights: MiniCPM-V 4.6 and RT-DETRv2 model support plus an Anthropic-style /v1/messages API endpoint (lablup#82, lablup#80, lablup#74); fixes for missing or null chat-message content, Gemma 3n VLM per_layer_inputs, Qwen3.5 MTP verify attention, and batched quantized KV cache mask offsets (lablup#91, lablup#86, lablup#78, lablup#76); and a pinned 1.93.1 Rust toolchain (lablup#87, lablup#90).
be-free-snowboy pushed a commit to freestylejs/snowbrain-mlxcel that referenced this pull request Oct 27, 2026
…y fix

The v0.0.31 changelog and debian entries described the Gemma 3n VLM per_layer_inputs fix as keeping per-layer input shapes correct, which undersells it: the real bug was a batch-scheduler concurrency race where a burst of concurrent VLM requests overwrote a single shared cell before prefill consumed it, reading the wrong sequence's tensor. Reword both entries to match the GitHub release note. The v0.0.31 tag is unchanged, so this lands on main for the next release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:inference Generation, sampling, decoding (incl. speculative, DRY) area:models Model architectures, weights, loading, metadata priority:high High priority status:done Completed type:bug Bug fixes, error corrections, or issue resolutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(vision): Gemma 3n VLM per_layer_inputs is not sequence-aware (concurrent-request race)

1 participant