Keep Video VAE RGB output finite for non-finite values - #9
Open
morluto wants to merge 1 commit into
Open
Conversation
morluto
marked this pull request as ready for review
August 11, 2026 10:48
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
h3_video_frames.rgbdocuments that decoded values are F32 in[0,1], butthe Video VAE output boundary currently clamps with two ordered comparisons:
Both comparisons are false for NaN, so a non-finite decoder value passes into
the returned RGB buffer unchanged. It can then propagate through tiled or
temporal blending and reach the final integer conversion.
Change
Clamp the decoded value with
fminf(fmaxf(value, 0.0f), 1.0f). The standardfloating-point min/max behavior selects the finite operand when only one
operand is NaN, making the output mapping deterministic:
0.0f-INFINITY0.0f0.0f+INFINITY1.0f1.0fFinite values already inside
[0,1]are unchanged, and ordinary out-of-rangevalues retain the existing saturation behavior.
Regression coverage
The real Video VAE test now has a focused
--nonfinite-outputmode. It sends aminimal
2x1x1all-NaN latent through the publich3_video_vae_decodeAPI andchecks that every returned RGB component is finite and within
[0,1].Without the new clamp, the NaN output fails that contract check.
Validation
fminf/fmaxfprobe returns0.0for NaN and negative infinity,and
1.0for positive infinity.git diff --checkpasses.