Skip to content

fix(tq): un-rotate GPU Lloyd-Max V aggregate (#435) + centralize KVarN support matrix (#437) - #438

Merged
pekkah merged 4 commits into
masterfrom
fix/435-437-tq-unrotate-support-matrix
Jul 12, 2026
Merged

fix(tq): un-rotate GPU Lloyd-Max V aggregate (#435) + centralize KVarN support matrix (#437)#438
pekkah merged 4 commits into
masterfrom
fix/435-437-tq-unrotate-support-matrix

Conversation

@pekkah

@pekkah pekkah commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Two #432 follow-ups. Both changes touch disjoint files and land as separate commits.

#435 — GPU Lloyd-Max attention un-rotates the compressed-region V aggregate

Both GPU Lloyd-Max TurboQuant attention kernels — CUDA llm_tq_attention and the Vulkan TqAttention shader — aggregated the compressed-region V contribution in the rotated/sign-flipped basis and never applied the inverse transform, then added it to the unrotated FP32-window contribution and wrote it straight to the output projection. The rotation R = D·H is orthonormal so K-scores are fine, but the V aggregate must be brought back: omitting R⁻¹ scrambles the compressed part of attention output by a fixed rotation. This corrupted generation for any model once context exceeded the 256-token FP32 window, on CUDA and Vulkan alike (independent of the quantizer distortion in #432).

Fix — Phase 3 now mirrors the in-tree correct pattern (TurboQuantKvCache.ComputeVAggregation / llm_kvarn_attention): accumulate the compressed V aggregate in the rotated domain, apply the deferred per-head sign-flip + inverse WHT once (reusing shared scratch), then add the FP32-window contribution in the original domain. The per-head sign patterns are threaded into both backends' TqAttention host methods and all four engine call sites (CudaForwardPass, CudaHybridForwardPass, GpuForwardPass, HybridForwardPass).

TqAttention_NeedleInHaystack is rewritten to compare against the original basis (a new ReconstructOriginal helper); it previously enshrined the wrong basis.

#437 — centralize the triplicated support matrix + extend to the server

The KVarN-vs-Lloyd-Max matrix was hand-copied across RunCommand, PerplexityCommand.ResolveAutoQuantizer, and InferenceEngineLoader.ResolveTq, with the #432 warning copied 4×. New SharpInference.Engine.TqSupport is the single source of truth (KVarNBlockedReason, head-dim envelope predicates, reason-string constants, QualityWarningReason); all three frontends delegate.

Server extensions:

  • Relaxed the up-front head-dim gate from a hard {128,256} reject to the codec-aware envelope, so KVarN's broader head dims (32/64/512/1024) are reachable on the server CPU path.
  • ResolveTq now throws a clean error instead of silently downgrading auto → Lloyd-Max on a head dim Lloyd-Max can't handle (would have crashed in the forward-pass ctor).
  • Rejects explicit TqMode=kvarn when TurboQuant=false (the CLI already did).

Validation

  • CUDA (RTX 4070): TqAttention_NeedleInHaystack passes against the original basis in the 100%-compressed region; all 5 CudaTurboQuantTests + 19 CudaKvarn + 78 TurboQuant codec tests pass. End-to-end Qwen3-0.6B --tq-mode lloydmax -g -1 stays coherent past the window and matches the CPU path over a 180-token generation with a large compressed region.
  • Centralize the KVarN support matrix + extend it to the server (TurboQuant head-dim gate) #437: 28 new TqSupportTests (incl. Vulkan / no-CUDA GPU sub-cases) + 33 unchanged Perplexity tests + 154 server tests pass. Release build clean under the AOT/trim analyzers.

⚠️ Follow-up required before merge (needs a Vulkan SDK)

The Vulkan TqAttention shader source changed, so its precompiled SPIR-V is stale. Run scripts/gen-spirv.ps1 on a machine with the Vulkan SDK (glslc) to regenerate Shaders.Precompiled.g.cs, then rebuild SharpInference.Vulkan. Until then, VulkanPrecompiledShaderTests.AllShaderConstantsArePrecompiled (hash drift) and VulkanShaderTests.TqAttention_LongContextScratchPath_MatchesFastPath ("glslc not found") fail by design of the drift guard — not a logic regression. The Vulkan Phase-3 code is a faithful port of the validated CUDA/CPU pattern.

🤖 Generated with Claude Code

pekkah and others added 2 commits July 11, 2026 21:16
… attention (#435)

Both GPU Lloyd-Max TurboQuant attention kernels (CUDA `llm_tq_attention` and the
Vulkan `TqAttention` shader) aggregated the compressed-region V contribution in the
rotated/sign-flipped basis and never applied the inverse transform, mixing it with
the unrotated FP32-window contribution. This scrambled attention output for ANY
model once context exceeded the 256-token FP32 window, on CUDA and Vulkan alike.

Rewrite Phase 3 to mirror the CPU `TurboQuantKvCache.ComputeVAggregation` and the
KVarN kernel: accumulate the compressed V aggregate in the rotated domain, apply the
deferred per-head sign-flip + inverse WHT once (reusing shared scratch), then add the
FP32-window contribution in the original domain. Thread the per-head sign patterns
into both backends' `TqAttention` host methods (CUDA arg array 16->17; Vulkan binding
9, count 9->10) and all four engine call sites.

Rewrite `TqAttention_NeedleInHaystack` to compare against the ORIGINAL basis via a new
`ReconstructOriginal` helper; the test previously enshrined the wrong basis.

Validated on CUDA (RTX 4070): needle test passes in the fully-compressed region;
end-to-end Qwen3-0.6B Lloyd-Max stays coherent past the window and matches the CPU
path. Vulkan is a faithful port of the same pattern but its shader source changed —
run `scripts/gen-spirv.ps1` on a Vulkan-SDK machine to regenerate
`Shaders.Precompiled.g.cs` before the Vulkan drift tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tend to the server (#437)

The KVarN-vs-Lloyd-Max TurboQuant support matrix — which combinations of (SnapKV,
head dim, backend, full-vs-partial offload, MoE, FP32 window) support KVarN — was
hand-copied across three frontends (RunCommand, PerplexityCommand.ResolveAutoQuantizer,
InferenceEngineLoader.ResolveTq), with the #432 quality warning copied 4x. A missed
edit silently resolved auto -> Lloyd-Max on one surface only, reintroducing the #432
collapse (the exact shape of the partial-offload crash fixed in #436).

Add `SharpInference.Engine.TqSupport` as the single source of truth: `KVarNBlockedReason`
(the auto matrix), head-dim envelope predicates (IsKVarNHeadDim pow2[8,1024],
IsKVarNCudaHeadDim pow2[8,256], IsLloydMaxHeadDim {128,256}), reason-string constants,
and `QualityWarningReason`. All three frontends now delegate.

Extend to the server:
- Relax the InferenceEngineLoader up-front head-dim gate from a hard {128,256} reject
  to the codec-aware envelope, so KVarN's broader head dims (32/64/512/1024) are
  reachable on the server CPU path, as they already are on the CLI.
- ResolveTq now throws a clean error instead of silently downgrading auto -> Lloyd-Max
  on a head dim Lloyd-Max has no codebook for (would have crashed in the fwd-pass ctor).
- Reject explicit TqMode=kvarn when TurboQuant=false (the CLI already did).

Unit-test the matrix once in TqSupportTests (28 cases incl. the Vulkan / no-CUDA GPU
sub-cases the Perplexity wrapper can't express). Existing Perplexity + server tests
unchanged and green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request centralizes the TurboQuant support matrix logic into a new TqSupport class to ensure consistent fallback and validation behavior across the CLI and server frontends. It also implements deferred sign-flipping and inverse Walsh-Hadamard Transform (WHT) directly within the CUDA and Vulkan attention kernels, resolving issue #435. The review feedback highlights a potential issue in PerplexityCommand.cs where hardcoding CUDA availability and GPU offloading assumptions could lead to incorrect fallback decisions on non-CUDA machines; querying CudaBackend.IsAvailable() dynamically is recommended to resolve this.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +143 to +145
fallbackReason = TqSupport.KVarNBlockedReason(
headDim, snapKvEnabled, onGpu: nGpuLayers != 0,
isVulkan: false, cudaAvailable: true, isMoE, window: tqWindow);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding cudaAvailable: true and onGpu: nGpuLayers != 0 can lead to incorrect fallback decisions on non-CUDA machines. If CUDA is unavailable, the execution will fall back to the CPU, where KVarN is fully supported for any power-of-2 head dimension up to 1024. By assuming CUDA is always available, a model with a head dimension greater than 256 will incorrectly fall back to Lloyd-Max even when running on the CPU. Querying CudaBackend.IsAvailable() ensures the fallback logic correctly identifies when CPU execution is active.

        fallbackReason = TqSupport.KVarNBlockedReason(
            headDim, snapKvEnabled, onGpu: nGpuLayers != 0 && CudaBackend.IsAvailable(),
            isVulkan: false, cudaAvailable: CudaBackend.IsAvailable(), isMoE, window: tqWindow);

pekkah and others added 2 commits July 11, 2026 21:23
…on (review)

Code-review nit on #435: the Vulkan TqAttention un-rotate used inversesqrt() for the
inverse-WHT normalization while the CPU WalshHadamard.Transform (which this inverts)
and the CUDA kernel use exact 1/sqrt(D). inversesqrt is a lower-precision approximation
on some GPUs and no Vulkan-vs-CPU parity test would catch the drift. Match the exact
convention (same as the TqRotateQuery shader).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… un-rotate

The TqAttention shader source changed in this PR (deferred per-head sign flip
+ inverse WHT of the compressed-region V aggregate), leaving its precompiled
SPIR-V blob and FNV hash key stale. Regenerated via scripts/gen-spirv.ps1
(Vulkan SDK 1.4.341.1); only the TqAttention entry changed (9668 -> 10972 bytes).

VulkanPrecompiledShaderTests (drift guard) and the on-device
VulkanShaderTests.TqAttention paths now pass, alongside the CUDA TQ/KVarN suites.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JNwRmR1tuioHTiiabU34Rp
@pekkah

pekkah commented Jul 12, 2026

Copy link
Copy Markdown
Owner Author

Pre-merge blocker resolved: regenerated Shaders.Precompiled.g.cs via scripts/gen-spirv.ps1 (Vulkan SDK 1.4.341.1) — only the TqAttention entry changed (9668 → 10972 bytes). Reviewed and verified locally:

  • Build: full solution Release, 0 warnings (TreatWarningsAsErrors + AOT/trim analyzers on).
  • Tests: 28 TqSupportTests + 3 VulkanPrecompiledShaderTests (drift guard) + 39 CUDA TQ/KVarN + on-device VulkanShaderTests.TqAttention — all pass.
  • Review: un-rotate math correct (per-head sign flip → inverse WHT → 1/√D, linear so aggregating-then-unrotating ≡ unrotating-then-aggregating; mirrors TurboQuantKvCache.ComputeVAggregation); _gpuSignPatterns wired through all four passes; shared-mem sdata[256] non-colliding with shared_scores; host↔kernel/shader arg order consistent; sign-pattern tensor layout [numKvHeads*headDim] matches the kv_head*head_dim+d indexing.

🤖 Generated with Claude Code

@pekkah
pekkah merged commit 27bd7a8 into master Jul 12, 2026
1 check passed
@pekkah
pekkah deleted the fix/435-437-tq-unrotate-support-matrix branch July 12, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant