Keep scaled queries in F32 for causal GQA - #4
Open
morluto wants to merge 1 commit into
Open
Conversation
morluto
marked this pull request as ready for review
August 11, 2026 09:39
AlexanderIstomin
added a commit
to AlexanderIstomin/h3.c
that referenced
this pull request
Aug 15, 2026
All four come from pull requests open against antirez/h3.c that have not been merged yet. Each was applied to this fork, built, and exercised here rather than taken on trust. Keep scaled queries in F32 for causal GQA (morluto, PR antirez#4). The kernel expanded a BF16 query to F32, applied the attention scale, then rounded the product back to BF16 before the QK contraction — into a threadgroup float array, so the rounding saved no storage and only discarded precision. Their regression test, included here, measures max absolute error 0.0039 and a 0.0076% BF16 mismatch rate on this machine, against 0.092 and 21.6% for the old rounding. This changes the numerics of every generation, which is why the test ships with it. Resolve a relative shader path against the executable's directory (guyz, PR antirez#14). Adapted rather than cherry-picked: upstream patches the inline path in h3_gpu_create, while this fork resolves paths in h3_gpu_shader_path, so the fallback lands there and covers the precompiled metallib too. Without it the binary only runs from the directory holding h3_shaders.metal. Clamp H3_VAE_TILE_PIXELS to 320 (guyz, PR antirez#14). Tiles above 320 produce a grid artifact across the frame. This fork's automatic search already stopped at 320; only the environment override could exceed it. Keep video VAE RGB output finite (morluto, PR antirez#9) and reject unrepresentable aligned frame counts (morluto, PR antirez#11). Both close paths where a bad value propagates instead of failing: NaN passes two ordered clamp comparisons untouched, and a large frame count overflows while being aligned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Fixes #3.
Why
The causal GQA Metal kernel expanded a BF16 query to F32, applied the attention scale, then rounded that product back to BF16 before performing the QK contraction in F32:
Because
shared_queryis already athreadgroup floatarray, the intermediate round does not save storage. It only discards precision before the scores and softmax are computed.At
head_dim=128, an A/B test of the real kernel over 64 deterministic trials and 2,097,152 outputs produced:The variants differed in 21.65% of output elements. Both were bit-for-bit deterministic.
What changed
bf16_to_f32(query) * scalein F32 through the QK contraction.head_dim=128and compares it with an explicit F32 causal-GQA reference.make testtarget.The test uses numerical tolerances rather than device-specific hashes. A mutation check confirmed that it passes the fixed code and fails when the old BF16 re-round is restored:
Performance
The real kernel was timed with 200 dispatches per sample, two series per variant, GPU command-buffer timestamps, and the first ramp-up sample in each series excluded.
The timing distributions overlap; the relevant conclusion is that no performance regression was observed.
Validation
Validated on Apple M5, macOS 26.3, arm64, Metal 4:
make -j"$(sysctl -n hw.logicalcpu)" h3_gqa_tests h3_audio_gpu_tests h3_tests ./h3_gqa_tests ./h3_audio_gpu_tests ./h3_tests git diff --checkh3_gqa_tests: passedh3_audio_gpu_tests: passedh3_tests: passed, 1,768 checksScope
This kernel is used by the Qwen3-VL prompt encoder. The DiT attention path uses MPSGraph SDPA and is unchanged.
This PR is scoped to the directly reproduced kernel-level precision loss and its regression coverage.