Repository navigation
vision, gpu-vision, gpu-gates, serve: EmbeddingGemma 2 image input through the Gemma4 vision tower - #324
Conversation
github-actions
Bot
added
dgx-spark
cd784b1 to
8455141
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: MUST_FIX: The change repeats shared validation and conversion logic and allocates coordinate buffers with no consumer.
Must fix
1. Reuse the media span checker
crates/gpu-gemma-embedding2/src/pass.rs:420 repeats the shape/order contract already owned by crates/gpu/src/media.rs:191 (R14).
Use the existing checker and widener so media refusals keep one owner. Replace lines 419–452 with:
bloomery_gpu::media::check_spans(PASS, spans, n, self.hp.n_embd)?;
let mut inp_scaled = self.token_rows(ids)?;
for span in spans {
let dst = &mut inp_scaled[
span.at.start * self.hp.n_embd..span.at.end * self.hp.n_embd
];
bloomery_gpu::media::widen_bf16(span.rows, dst);
if dst.iter().any(|v| !v.is_finite()) {
return Err(GpuError::shape(PASS, "image row contains a non-finite value"));
}
}2. Own the tap conversion once
crates/gpu-gates/src/bin/shared/vision_taps.rs:151 and crates/gpu-gates/src/bin/gate_vision_encoder.rs:454 add identical F32-to-BF16 adapters (R14).
Give the existing TapSink::take_f32 method this default and delete both adapters. This removes two definitions and prevents conversion fixes from diverging:
fn take_f32(&mut self, name: &str, cols: usize, values: Vec<f32>) {
self.take(name, cols, values.into_iter().map(crate::f32_bf16).collect());
}3. Remove the unused coordinate arrays
crates/vision/src/arch/gemma4v/media.rs:48 computes two coordinate vectors and immediately drops them. The device computes its own coordinates.
This adds two allocations and 8 bytes per patch per image [derived]. Remove positions, its re-export and its dedicated assertions now, before callers depend on this unused API. Replace preprocess with:
pub fn preprocess(image: &Rgb8, rule: &SizeRule) -> Result<Patches, VisionError> {
image.check()?;
let (best_w, best_h) = rule.aligned(image.width, image.height)?;
let (nx, ny) = rule.grid(best_w, best_h);
if nx.checked_mul(ny).is_none_or(|n| n > rule.max_tokens()) {
return Err(VisionError::Limits {
detail: "gemma4v aligned canvas exceeds the image token budget".into(),
});
}
let plan = GridPlan {
n_llm_h: ny,
n_llm_w: nx,
best_h,
best_w,
};
let resized = crate::resample::pad_ceil(image, best_w, best_h, [0; 3])?;
let patches = crate::preprocess::patchify(
&resized,
plan,
crate::preprocess::PatchLayout::row_major(rule.patch()),
)?;
if patches.n_vit_h > 10240 || patches.n_vit_w > 10240 {
return Err(VisionError::Limits {
detail: "gemma4v canvas exceeds the learned position table".into(),
});
}
Ok(patches)
}Non-blocking
⚠ Exercise HTTP image execution: crates/gpu-gates/src/side_seat/gemma_embedding2.rs:127 is bypassed by both numerical image gates; the mock test stops after parsing.
Replacing this method with an error leaves those checks green. Add an image request to the serving gate, assert its vector, and show FAIL-first for that mutant. This protects the endpoint wiring.
⚠ Supply the shared GEMM move proof: crates/gpu-vision/src/gemm_bf16.rs:95 moves the accumulator used by existing towers into a macro.
The body lists spill checks without an identical-PTX comparison. Attach just ptx-compare <base> gate_vision_encoder vision and the owning gates' results to distinguish shared-kernel regressions from new-tower failures.
Checked
Duplicates: 122 new Rust/script items checked; findings above. All 6 dryer pairs read: short preparation glue, distinct module loaders and error constructors; no extraction with fewer elements.
Complexity: new Gemma4 load/Scratch::new/encode_tapped, tower run, e2e images, pass_media, GEMM enqueue_f32, pointwise enqueue/positions and ScratchLens::new are load/validation/execution pipelines. Flags::take/read add option/projector branches; arch::open, e2e run and GLM clauses add dispatch/clause arms.
Silent failure: 10 sites followed; optional defaults are defined, malformed values reach named errors, and device accesses have checked extents.
Tests: static mutation audit covers span offsets, activation defaults, pixel/channel order, reference-device identity and scan architecture assertions; image-splice removal reaches numerical comparisons. HTTP execution gap noted above.
Gates: no numerical band relaxed; PTX and unsafe ratchet additions have dated pins.
Escape catalog: C1–C4 checked; none matched.
Notes
No build or GPU gate was run during this review. The body reports earlier heads; image e2e, GPU FAIL-first and affected-gate results remain pending.
Card check: gpu-evidence. This pull request changes crates/gpu-gates/Cargo.toml, crates/gpu-gates/src/arch/embedding.rs, crates/gpu-gates/src/bin/bloomery_serve.rs … and carries a gate run on the author's card, which counts as verified.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
github-actions
Bot
added
gpu-evidence
Vision and the text encoders share the named tensor and shape error contracts.
Extract the existing tensor-core staging and accumulation into one macro. The BF16 entry keeps its epilogue and launch contract; no F32 entry or launcher is added here.
A media rule may return no chat placeholder. Chat expansion, per-layer token checks and decision encoding refuse that case by name; the existing image models keep their token ids.
Read the Gemma4 header, activation and pooling scale, plan its card bytes and prepare raster patches on the shared dynamic size rule. Embedding images use text delimiters and have no chat placeholder.
Keep the convolution, learned positions, residual stream and post norms in F32, with BF16 transformer linears and raster pooling. Add the F32 GEMM store and explicit F32 tap handlers; row operations expose only the modes their launcher emits.
Budget the image tower before chat placement and run its rows with ordered text parts in one embedding pass. The HTTP route and gates share serve::embeddings::assemble. Build the encoder explicitly for serving instead of enabling it for every DeepSeek gate, and refuse duplicate side flags by name.
Register the tower and server CPU families, collect image-only and text-first vectors, and keep CPU server logs apart from card logs. The collector also compares a running HTTP server with saved llama-server vectors, printing cosine and maximum absolute difference.
Check F32 convolution pixels, raster pooling, every layer and projected rows. Image-only and mix_free-448x448 inputs use the serving assembler and compare raw and normalized vectors with both server sets. Reuse the family opener and the shared CPU-twin multiplier. PIN(2026-10-15): Remove the duplicate embd comparison because projected reads the same output buffer; the projected comparison keeps its band.
Document the image side seat, input parts, reserve and reference gates, with a command for comparing a running HTTP server to saved llama-server vectors.
Allow the architecture spellings whose occupancy limits ptx-scan already defines. This changes no server collection path.
Carry the measured Gemma4 entry rows for the default table and both architecture-specific PTX tables.
Regenerate the gate path table against the embedding seat's current encoder and keep the no-argument e2e recipe in step with its binary. Format the merged reference families and shared header fixture.
8455141 to
c83b590
Compare
There was a problem hiding this comment.
Verdict: CLEAN: No must-fix defect survived the span, buffer, numeric and error-path checks.
Non-blocking
⚠ Complete the pending GPU evidence: crates/gpu-gates/src/bin/gate_gemma_embedding2_e2e.rs:345.
The body still marks image end-to-end, GPU FAIL-first and affected-gate runs as pending. Add their commands, heads and final output. Recording the results now makes later regressions attributable.
⚠ Show the shared GEMM move proof: crates/gpu-vision/src/gemm_bf16.rs:94.
The extracted accumulator still feeds the existing towers' vis_gemm_bf16, with no launch-count change. The move class requires unchanged PTX for existing entries. Attach a base/head scan or canonical comparison; spill counts alone do not establish equivalence.
Optimization headroom
- G. Per-launch and per-sync fixed cost: crates/gpu-vision/src/arch/gemma4v.rs:609. Compute the group mean inside the existing pooling gather, preserving its rounding. Removing the intermediate buffer saves its BF16 write and read: 9 × 768 × 2 × 2 = 27,648 bytes per pooled token at width 768, plus one launch per image [derived].
Checked
Duplicates: 120 new declarations Grepped. All six dryer pairs read: short preparation adapters, module loaders and distinct error constructors; no extraction that reduces net elements.
Complexity: new Gemma4 load/encode_tapped/Scratch::new, tower::run, e2e::images, pass_media, ScratchLens::new, GEMM enqueue_f32 and pointwise::{enqueue,positions} contain load, stage or validation branches. Flags::{take,read} add image/context handling. arch::open, e2e::run and GLM clauses add dispatch or stages. No complexity-only finding.
Silent failure: 11 candidate sites traced to consumers; invalid inputs receive named refusals or guards prove the skipped case impossible.
Tests: static mutation review covers lost model flags, zeroed image offsets, eager image preparation, changed activation, wrong CPU family, deleted span checks and restored architecture refusal. Their assertions detect those mutations. GPU FAIL-first execution remains pending in the body.
Gates: dated reasons accompany PTX pins, unsafe-count increases and the scan architecture change. No numerical band relaxed.
Escape catalog: no C1–C4 match in added code.
Adversarial checks: empty and oversized inputs, overlapping spans, non-finite weights, missing chat tokens and partial output windows are refused or bounded. Independent gather/permutation and scratch-byte checks passed.
Notes
Static review only; no Rust build or GPU gate was run here.
Card check: gpu-evidence. This pull request changes crates/gpu-gates/Cargo.toml, crates/gpu-gates/src/arch/embedding.rs, crates/gpu-gates/src/bin/bloomery_serve.rs … and carries a gate run on the author's card, which counts as verified.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
|
The review is done and the contribution check passed, so I'm merging this. |
Part of #59 (plan and PR order in #190), E7: image input for EmbeddingGemma 2, through the Gemma4 vision tower and the embedding side seat. Builds on #299 and #323, both merged. E7's own twelve commits start after
gpu-gates: every gate's file digest is the crate's sha256.What and why
Release notes: Added: the embeddings endpoint takes images for EmbeddingGemma 2 (
--embd-mmproj), next to a chat model.EmbeddingGemma 2 embeds text and images into one vector space. Mainline llama.cpp runs the image half through
mtmdwith the model's projector file (mmproj-embeddinggemma-2-*.gguf): a Gemma4 vision tower whose output rows go into the text encoder in place of image placeholder tokens. E6 serves the text half beside a chat model. This PR adds the image half to the same side seat:crates/vision/src/arch/gemma4v/). Themtmdcanvas: the image is resized to a dynamic size the tower's patch grid divides, normalized, and given raster positions. The dynamic size rule is shared with the existing vision models (c563d901) instead of a second copy.crates/gpu-vision/src/arch/gemma4v.rs). Built from the shared tower kernels: the F32 patch convolution, learned position tables, BF16 attention and FFN blocks, and a 3×3 average pooling with a scale. Residuals, post-GEMM outputs and post norms stay F32, as mainline keeps them (d3f9b94c); the BF16 GEMM bodies are shared with the other towers through one macro. With no activation keys the tower follows mainline's Quick GELU;clip.use_gelu=trueselects tanh GELU and SiLU is refused.crates/gpu-gates/src/side_seat.rs,crates/serve/src/embeddings.rs). An input's image parts become rows of the encoder's single pass, between the delimiter tokens mainline's server writes. Tower and encoder share the side seat's card and thread with streams of their own, and their bytes are counted together before the chat placement.docs/serving.mdgains "EmbeddingGemma 2 images": the flags, the request shape and what is refused.A projector with Q8_0 vision matrices is refused by tensor name; the vision matrices must be BF16, which is what ggml-org publishes beside the Q8_0 text file.
Request shape and limits
A server started with
--embd-modeland--embd-mmprojtakes one object per input in theinputarray of/v1/embeddings, with an orderedcontentlist of text andimage_urlparts. An image URL is a base64data:URL for PNG or JPEG, as on the chat path; HTTP URLs are refused. A request with no loaded tower, with an unsupported image part or beyond the side context is a 400.--embd-hf <repo>[:<quant>]picks the row's BF16 projector,--embd-no-mmprojturns that off, and--embd-image-min-tokensand--embd-image-max-tokensset the image token budget apart from the chat model's flags.docs/serving.mdhas the full request, the flags and the refusals.The chat image path keeps its behavior. One change reaches it: a media rule may now return no chat placeholder, because an embedding image has none. Chat expansion, the per-layer token checks and the decision encoding refuse that case by name, and the existing image models keep their token ids (
vision, serve: represent a missing chat image token).Code shape
bf16_accumulate, shared with the other towers). The HTTP route and the gates build an input's rows through one function,serve::embeddings::assemble. The tower crates use the common shape and tensor errors.crates/vision/src/arch/gemma4v/; the seat has no branch per model.cargo fmtis clean (under "What I ran").What I ran
The head is
55156aa2, a rebase of the previous headbda0b1c9onto mainb9808a65with no conflicts. The range-diff shows all twelve of our commits as unchanged (=), which is why thecontrib-checkresult below, run on the earlier head83eab52b, is carried over. The card runs and host checks used DGX Sparks in the project's dev container (bloomery-dev:cuda13.3.1, imagesha256:853c20d59e68), CUDA 13.3.1,nvidia-smi. Each row names the head its result is from:After the review of
84551417, image media spans go throughgpu::media's span checker, one helper owns the tap conversion, and the unused coordinate arrays are gone. At head710838ce,/v1/embeddingschecks each image's span while it assembles the input and stops at the first image past the context, so a request with many images is refused before they are all prepared; both image gate recipes select thegemma-embedding2model. At that headcargo test -p bloomery-serve embeddingspassed (7 tests), and the new refusal test fails on the previous eager path.After the review of
710838ce, each memory access invis_clamp_f32,vis_add_f32,vis_pool_order_f32andvis_rms_norm_f32(each scalar read too) is its ownunsafeblock with its own SAFETY line, and the gpu-vision count intools/unsafe-ratchet.txtis recounted (116 to 127) with a dated PIN reason. The PTX of the tower gate is unchanged on sm_121 and sm_86.a688accb)cargo fmt --all -- --check1d71fd07)bloomery-serve,bloomery-visionandbloomery-refset1d71fd07)just contrib-check --tier t083eab52b(range-diff above)./tools/mac-static.sh(Mac)1d71fd07)check-recipesstep of./tools/mac-static.sh(Mac)1d71fd07)bloomery-gpu-gates8cb20c3f)just check-no-vision8cb20c3f)just gate-ptx-spill(cc 12.1), the Gemma4 tower8cb20c3f)just gate-ptx-spill(cc 12.1),generate_ds41andgate_e2e8cb20c3f)8cb20c3f)8cb20c3f)just gate-gpu-gemma4v-tower55156aa2: 133/133 numerical rows, 14/14 bit checksjust gate-gpu-gemma-embedding2-e2e, image inputsjust gate-gpu-gemma-embedding2-serve, image requests55156aa2: 188/188 checks, 181 vector comparisonsjust affectedselectsThe Mac's
check-recipesis red withFAIL cold run: the base side runs in a temp archive, away from this tree's gitand with theptx-compareself-test, which fails 1 of 33 cases. Maina688accbgives the same two lines on the same Mac, and the 59 lint warnings come from the same source files on both sides. I did not look for the causes of these reds.The three CPU-side edits were:
f.model = model;replaced byf.model = source::Flags::default();incrates/gpu-gates/src/side_seat.rs;image.at = span.start;replaced byimage.at = 0;incrates/serve/src/embeddings.rs; andKind::ServerCpu,replaced byKind::Server,incrates/refset/src/arch/gemma_embedding2/mod.rs.Comparison with the reference
The reference is mainline llama.cpp
6868a52cb337450658006ad2f9281c2eb63f1666. The comparison runs the same Q8_0 model, BF16 projector and seven fixed images through the card and the CPU twin, with the gates and bands unchanged.All tower rows stayed within 2.5 times the CPU/card twin distance, and all 181 serving vector comparisons stayed within their unchanged derived cosine and absolute-error bands.
Hardware (if the change is for a machine)
DGX Spark:
NVIDIA GB10, compute capability 12.1, driver 595.84, dev containernvidia/cuda:13.3.1-devel-ubuntu24.04. RTX 5090:NVIDIA GeForce RTX 5090,sm_120.What I ran into
None of this blocks the PR.
a688accb. They are the Mac's lint count, the Mac'scheck-recipesstep and the 11 unpinned PTX entries ofjust gate-ptx-spill. Each gives the same result with and without this PR, so I left them alone.passage-tokensfailed in upstream CI once and did not reproduce. On the previous PR head84551417,check-recipesincontrib-checkfailed withFAIL passage-tokens: no 'B2 passage 120 tokens' in the happy run's output. I ran the same self-test (tools/ref/q38-3090-server.sh --self-test) ten times in a row on Linux aarch64 at55156aa2, and it passed every time. The script is byte-identical on that PR head, this head and upstream main. I have not found the cause and have not run it on x86_64.AI help
AI 코딩 에이전트가 코드를 작성한 후 설명을 썼고, 테스트를 마쳤습니다. diff 리뷰와 PR 초안 리뷰는 제가 직접 했습니다.