Avoid overflow when aligning large frame counts - #11
Open
morluto wants to merge 1 commit into
Open
Conversation
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.
Problem
Frame requests are aligned upward to the next legal
5 + 17nshape. The calculation currently uses signedintarithmetic even though CLI and library inputs can reachINT32_MAX.For
INT32_MAX, the required calculation is:The result cannot fit in an
int. In an executed probe compiled from the upstream helper, it wrapped to-2147483635instead of producing a valid aligned count.Change
Perform the alignment arithmetic in
int64_t. If the next legal frame count cannot be represented by the function'sintreturn type, return zero as an explicit failure sentinel. Generation parameter validation now rejects that sentinel with the existing frame-range error.Zero is unambiguous because every valid aligned frame count is at least 5.
Regression coverage
The host test now checks that aligning
INT32_MAXreturns zero. The same probe compiled from the candidate helper returns zero, while the upstream helper fails that expectation.Validation
-2147483635, candidate0h3.candtests/test_h3.c— passedgit diff --check— passed