Repository navigation
fix(#1311 cluster-3): snap VLM vision-encoder head count to divide visionDim cleanly - #1397
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughLayerHelper centralizes MultiHeadAttention configuration by adding ChangesMultiHeadAttention Configuration Centralization
Sequence DiagramNo sequence diagram required—the changes are a single-file refactor without multi-component sequential interactions. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…sionDim cleanly PR #1290 CI Cluster 3 #1311: 23 SmolVLM tests failing on master with the cluster's signature shape-mismatch: System.ArgumentException : Input embedding dimension (384) does not match weight dimension (378). Query shape: [1, 256, 384], Weights shape: [378, 378] ## Root cause SmolVLM defaults: VisionDim=384, NumHeads=9. At the vision-encoder MHA construction in `CreateDefaultPixelShuffleProjectorLayers` (and 9 other VLM factories): new MultiHeadAttentionLayer<T>(numHeads > 16 ? 16 : numHeads, (visionDim) / (numHeads > 16 ? 16 : numHeads)) C# integer division: `384 / 9 = 42`. Then `MultiHeadAttentionLayer._embeddingDimension = 9 * 42 = 378` (NOT 384). The QKV weight matrices end up sized `[378, 378]`, but `PatchEmbeddingLayer` upstream emits patch tokens at visionDim=384 — so `ForwardInternal` throws at the very first vision MHA call. The 9-heads / 384-vision-dim mismatch is paper-faithful (SmolVLM uses SmolLM's 9-head decoder config) but the vision encoder is SigLIP-Large @ 16 heads × 64 head-dim = 1024 vision-dim — different counts per subsystem. AiDotNet's `SmolVLMOptions` collapses both to a single `NumHeads` knob, so the factory reuses the decoder's 9 for the vision MHA where it doesn't divide. Per-subsystem head counts on the options class (`NumVisionHeads` vs `NumDecoderHeads`) is the paper-faithful long-term fix but is an API-surface change. The minimal, no-surface-change fix is to snap the vision MHA's head count downward to the largest divisor of visionDim that's ≤ numHeads. ## Fix Add `ChooseDivisibleHeadConfig(embedDim, requestedHeads, maxHeads = 16)` helper in `LayerHelper<T>` that returns `(heads, headDim)` with `heads * headDim == embedDim` exactly — finds the largest `h ≤ min(requestedHeads, maxHeads)` such that `embedDim % h == 0`. For SmolVLM (visionDim=384, numHeads=9): start at 9, 384%9=6≠0, drop to 8, 384%8=0 ✓ → `(8, 48)`. MHA gets [384, 384] weights matching the 384-dim input. Add `CreateVisionMha(visionDim, numHeads, initializationStrategy?)` shim that applies the helper and returns the configured `MultiHeadAttentionLayer<T>`. Replace all 10 inline `new MultiHeadAttentionLayer<T>(numHeads > 16 ? 16 : numHeads, ...)` call sites across the VLM factories. Snapping heads downward (vs upward / padding embedDim) keeps every other shape in the chain unchanged — FFN, LayerNorm, downstream Dense all keep their visionDim-wide view. The trade-off is the attention pattern uses slightly fewer heads than the upstream model card; that's strictly more local than reshaping the entire residual stream. ## Verification Pre-fix (current master): $ dotnet test --filter "FullyQualifiedName~EmotiVoiceTests|FullyQualifiedName~Phi3VisionTests|FullyQualifiedName~SmolVLMTests|FullyQualifiedName~RainbowDQNAgentTests" Failed: 47, Passed: 37 EmotiVoiceTests: pass=26, fail=1 (timeout) Phi3VisionTests: pass=2, fail=23 (all OOM/timeout, foundation-scale) RainbowDQNAgentTests: pass=7, fail=0 SmolVLMTests: pass=2, fail=23 (all shape-mismatch — THIS PR) Post-fix: $ dotnet test --filter "FullyQualifiedName~SmolVLMTests" Failed: 14, Passed: 11 Remaining 14 failures: 7 OutOfMemoryException + 6 timeout 120s + 1 timeout 180s — NO MORE shape mismatch. So this PR closes **23 of 23 SmolVLM shape-contract failures**. The remaining 14 SmolVLM failures (plus Phi3Vision's 23) are foundation-scale resource issues — same class as #1394 (ResNet/VGG ImageNet-scale perf). Different root cause, separate follow-up. ## Affected paths (10 sites) All `(visionDim) / (numHeads > 16 ? 16 : numHeads)` patterns in VLM factories: - CreateDefaultEncoderDecoderVLMLayers - CreateDefaultVisualExpertVLMLayers - CreateDefaultCrossAttentionResamplerVLMLayers - CreateDefaultPixelShuffleProjectorLayers (SmolVLM — direct fix here) - CreateDefaultVisionAdapterLayers (Phi3Vision) - CreateDefaultTokenReductionVLMLayers (DeepSeek-VL) - + 4 more Closes #1311 partially (shape-contract root cause for SmolVLM; defensive fix applied to all 10 vision-encoder MHA sites). Foundation-scale resource residue tracked elsewhere.
c074340 to
8e17931
Compare
Summary
Closes the shape-contract root cause of #1311 (PR #1290 CI Cluster 3 — VLM/Multimodal cross-attention embedding-dim mismatch). 23 of 23 SmolVLM shape failures eliminated. The other affected models (EmotiVoice, RainbowDQN) were already fixed by intervening work; Phi3Vision's remaining failures are foundation-scale OOM/timeout (separate class).
Per-model state on current master (pre-fix in this PR)
Root cause (SmolVLM)
SmolVLM defaults: `VisionDim=384, NumHeads=9`. At the vision-encoder MHA construction in `CreateDefaultPixelShuffleProjectorLayers` (and 9 other VLM factories):
```csharp
new MultiHeadAttentionLayer(numHeads > 16 ? 16 : numHeads,
(visionDim) / (numHeads > 16 ? 16 : numHeads))
```
C# integer division: `384 / 9 = 42`. Then `MultiHeadAttentionLayer._embeddingDimension = 9 * 42 = 378` (NOT 384). The QKV weight matrices end up sized `[378, 378]`, but `PatchEmbeddingLayer` upstream emits patch tokens at visionDim=384 — so `ForwardInternal` throws at the very first vision MHA call with:
```
System.ArgumentException : Input embedding dimension (384) does not match
weight dimension (378). Query shape: [1, 256, 384], Weights shape: [378, 378]
```
The 9-heads / 384-vision-dim mismatch is paper-faithful (SmolVLM uses SmolLM's 9-head decoder config) but the vision encoder is SigLIP-Large @ 16 heads × 64 head-dim = 1024 vision-dim — different counts per subsystem. AiDotNet's `SmolVLMOptions` collapses both to a single `NumHeads` knob, so the factory reuses the decoder's 9 for the vision MHA where it doesn't divide.
Fix
Add `ChooseDivisibleHeadConfig(embedDim, requestedHeads, maxHeads = 16)` helper in `LayerHelper` that returns `(heads, headDim)` with `heads * headDim == embedDim` exactly — finds the largest `h ≤ min(requestedHeads, maxHeads)` such that `embedDim % h == 0`. For SmolVLM (visionDim=384, numHeads=9): start at 9, 384%9=6≠0, drop to 8, 384%8=0 ✓ → `(8, 48)`. MHA gets `[384, 384]` weights matching the 384-dim input.
Add `CreateVisionMha(visionDim, numHeads, initializationStrategy?)` shim that applies the helper and returns the configured `MultiHeadAttentionLayer`. Replace all 10 inline `new MultiHeadAttentionLayer(numHeads > 16 ? 16 : numHeads, ...)` call sites across the VLM factories.
Snapping heads downward (vs upward / padding embedDim) keeps every other shape in the chain unchanged — FFN, LayerNorm, downstream Dense all keep their visionDim-wide view. The trade-off is the attention pattern uses slightly fewer heads than the upstream model card; that's strictly more local than reshaping the entire residual stream. Per-subsystem head counts on the options class (`NumVisionHeads` vs `NumDecoderHeads`) is the paper-faithful long-term fix but is an API-surface change; the minimal no-surface-change fix is to snap downward.
Affected factories (10 sites all converted)
All `(visionDim) / (numHeads > 16 ? 16 : numHeads)` patterns: `CreateDefaultEncoderDecoderVLMLayers`, `CreateDefaultVisualExpertVLMLayers`, `CreateDefaultCrossAttentionResamplerVLMLayers`, `CreateDefaultPixelShuffleProjectorLayers` (SmolVLM — direct fix here), `CreateDefaultVisionAdapterLayers` (Phi3Vision), `CreateDefaultTokenReductionVLMLayers` (DeepSeek-VL), + 4 more.
Defensive fix: applies to any VLM where the configured `NumHeads` doesn't divide `VisionDim` cleanly — not just SmolVLM's specific 9/384 mismatch.
Verification
Pre-fix on master:
```
$ dotnet test --filter "FullyQualifiedName~SmolVLMTests"
Failed: 23, Passed: 2
All 23 failures: "Input embedding dimension (384) does not match weight dimension (378)"
```
Post-fix:
```
$ dotnet test --filter "FullyQualifiedName~SmolVLMTests"
Failed: 14, Passed: 11
Remaining 14: 7 OutOfMemoryException + 6 timeout 120s + 1 timeout 180s
NO MORE shape mismatch failures.
```
This PR closes 23 of 23 SmolVLM shape-contract failures. Remaining 14 SmolVLM + 23 Phi3Vision failures are foundation-scale resource issues (OOM at ImageNet-scale ViT-Large + multi-billion-param decoders), same class as #1394 (ResNet/VGG ImageNet-scale perf). Different root cause; separate follow-up.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit