fix(abot): restore prompt-pad zeroing, report live prompt rows, cut walk overhead - #37
Open
DmitryMalishev wants to merge 6 commits into
Open
fix(abot): restore prompt-pad zeroing, report live prompt rows, cut walk overhead#37DmitryMalishev wants to merge 6 commits into
DmitryMalishev wants to merge 6 commits into
Conversation
The August forward-port (651de1a) rewrote sd_abot_scene_create against the new AbotT5Runner/T5Embedder API and dropped the reference WanTextEncoder's `u[v:] = 0` semantic that the July implementation carried via T5CLIPEmbedder's zero_out_masked. The tokenizer's additive attention mask only masks attention *inside* the encoder; the encoder still emits live embeddings for all 512 positions, so packs written since the port carry ~500 rows of pad-token embeddings in prompt_embeds. The walk DiT cross-attends to the full 512-row context, the padding rows swamp the conditioning, and every walk from a newly created scene collapses into blur/garbage within the first block. Packs created before the port keep working, which is why the walk path itself validated clean. Zero the output rows whose mask is not 0.0f (-inf = padding) right after the encode, restoring parity with the reference pipeline. Verified on the RTX 5090 host: fixed packs carry 15 non-zero rows for the 15-token test prompt (was 512), real-token rows byte-identical to pre-regression packs, and a 6-block Vulkan walk from a fixed pack holds luminance stddev 41-46 (regressed packs collapse to 8-10 by block 1) on both the recompute and KV-cache paths, CPU and Vulkan backends. Root-caused by A/B-ing addon 0.19.0/0.21.0 with crossed scene packs: any engine walking a 0.21-created pack produced mush, both engines walking a 0.19-created pack stayed clean, and per-tensor pack comparison showed prompt_embeds cos=0.17 / norm 17->101 with every other tensor bit-equal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 2026-08-11 port regression (fixed in the previous commit) was silent: a pack whose prompt padding was never zeroed loads fine, walks fine, streams the right number of correctly sized frames, and only betrays itself in the pixels. The information that would have named the bug immediately - how many of the 512 context rows actually carry a prompt - was available at load time and not reported. Count the rows before the zeroed padding when a pack loads. A healthy pack logs `prompt rows 22 live / 512`; a pack whose padding is not zeroed warns and names the likely cause, because its walk is conditioned on pad-token embeddings. Verified against both: the 0.21.0-produced pack warns, packs from the fixed producer report 22/512. Measurements behind the wording, on an RTX 5090 (Vulkan, Q8 DiT, 832x480, same image and seed, only the prompt differing): - Prompt conditioning is alive and reference-faithful. Two very different prompts produce visibly different walks (mean |pixel diff| 69.6/255, 99.3% of pixels differing), and re-encoding the same prompt is bit-identical. - Text is nonetheless weak against a first-frame image, and that is architectural, not a defect: the reference passes all 512 rows to cross-attention with `context_lens = None` and no mask (wan/modules/causal_model.py:2180 and attention.py:211 in amap-cvlab/ ABot-World), runs no classifier-free guidance anywhere, and feeds the image through much stronger channels (first-frame latent replacement plus reference tokens inside self-attention). So a prompt shifts a walk's trajectory and tone rather than replacing scene content. - Trimming the ~490 zero rows so the real tokens get the full attention mass looks like the obvious way to strengthen text. It was measured and it destroys generation: output collapses to near-black mush within one block. Those rows are load-bearing - the model was trained with a fixed 512-row context - which is also why this commit only reports the count and changes no behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two changes to the interactive walk, both bit-identical in output and both
confined to the ABot path (`forward_kv` is reached only from
`forward_cached`, which only `AbotWorldRunner` calls, so no other model's
attention is touched).
1. The walk ran `GGMLRunner::compute(..., auto_free=false)` and left
`free_compute_buffer` at its default `true`, so every one of the 5-6 graphs
per block destroyed the graph allocator and its multi-GB VRAM buffer, and
the next graph re-created and re-reserved them. The walk is an endless
sequence of near-identical graphs, so the buffer is now retained;
`ggml_gallocr_alloc_graph` re-reserves on its own when the shape changes
between the denoise and append graphs (ggml-alloc.c:1064). Params are kept
staged across steps for the same reason.
2. The masked self-attention composed `scale -> add(mask) -> soft_max` as
three ops. The score tensor is the largest in the graph (T_kv x n_token x
heads, ~625 MB at 832x480 with a full history ring), so each elementwise
pass costs a full read and write of it. `ggml_soft_max_ext` does all three
in one pass and supports the walk's {T_kv, n_token} mask broadcasting over
heads on CPU, CUDA and Vulkan (ggml.c:4022 asserts, plus the head modulo in
ggml-cuda/softmax.cu:69 and vulkan-shaders/soft_max.comp:51). The
composition it replaces predates that support.
Measured on an RTX 5090, Vulkan, Q8 DiT, 832x480, KV cache on, 12 frames per
block, ggml at the pinned 7d9ce11:
per block baseline +buffer +fused softmax
denoise step 310 ms 289 ms 241 ms
append pass 405 ms 372 ms 329 ms
block (steady) 2.1 s 2.0 s 1.8 s
block 0 (cold) 12.0 s 2.2 s 2.2 s
Frames are byte-identical to the baseline at every stage (mean |diff| 0.00 over
832x480 RGB, checked at blocks 1 and 3), so this is pure overhead removal, not
a quality trade. The cold-start collapse from 12.0 s to 2.2 s is the same
cause: block 0 builds several differently shaped graphs and was paying a full
reserve for each.
Remaining gap to the PyTorch reference (~0.42 s/block) is Q8 GEMM throughput,
not overhead: ~8.7 TFLOP of projection and FFN work per denoise step against
~2.4 TFLOP of attention. Closing it needs fp8/int8 GEMM kernels, a flash-
attention branch in `forward_kv` (which has none today), or a contiguous KV
ring to retire the per-layer concat chain - none of them a small change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two bit-identical wins in the walk, found by profiling a block stage by stage
on an RTX 5090 (Vulkan, Q8_0 DiT, 832x480, KV cache, 12 frames per block).
**Action planes.** The keyboard action is expanded into a {52, 30, 8192, 3}
F32 tensor - about 153 MB - and it was rebuilt on the host before every graph.
Within one block the key mask, the frame count and the latent size never
change, so four of the five fills per block were re-deriving identical bytes,
single-threaded, on the critical path ahead of each denoise step. The filled
buffer is now kept and rebuilt only when its key actually changes.
**Pixel decode.** The taehv decoder is a stack of small 3x3 convolutions, and
im2col+GEMM materializes a large intermediate for each one. The engine already
has a direct-convolution path (`set_conv2d_direct_enabled`, used for the VAE,
ESRGAN and ControlNet in the batch pipeline); the ABot session never enabled it
for its decoder.
per block before after
taehv decode 148 ms 68 ms
block (steady) 1.5 s 1.2 s
The action-plane change is a pure host-side saving, which is why the block
falls further than the profiled stages do: the refills sat in the gap between
stages that the [prof] lines do not cover. Frames are byte-identical to the
previous build at every stage checked (mean |diff| 0.00 over 832x480 RGB), so
neither change trades quality.
Two other candidates were tried and reverted rather than shipped, since both
lost time: direct convolution for the DiT itself (its patch embed and action
adapter got slower, 207 -> 211 ms per denoise step) and an F16 attention mask
(bit-identical, but converting 6.5 M values on the host each step cost more
than the halved upload and per-layer reads saved, 1.2 -> 1.3 s per block).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The walk's self-attention asked for `GGML_PREC_F32` on the Q·Kᵀ matmul. On a
coopmat2 device that is the worst of the three options: both operands are F32,
so the Vulkan backend takes the `full_f32` branch, and the only F32
cooperative-matrix pipeline (`pipeline_matmul_f32_cm1`) is built in the
coopmat1 branch that a coopmat2 device never enters. The request therefore
resolves to `matmul_f32_f32_fp32` - a scalar shader with no tensor cores - and
additionally suppresses the F32→F16 operand reformat. Dropping the flag routes
the same matmul to `matmul_f16_f16acc_cm2`.
This is the largest single matmul in the graph: at 832x480 with a full history
ring the scores are 5570 x 1170 x 24 (~625 MB), and it runs in all 30 layers of
every denoise and append graph.
per block before after
denoise step 241 ms 207 ms
append pass 329 ms 296 ms
block (steady) 1.8 s 1.6 s
The flag was inherited from the shared attention helper's convention rather
than from a measured ABot need, and the scores feed straight into a softmax,
so the accumulator's dynamic range is not what limits this operation.
Unlike the other perf changes on this branch, **this one is not bit-exact** -
it moves the accumulation to F16. Measured on the same scene and seed: a frame
inside block 2 differs by mean |diff| 0.29/255 with 0.2% of pixels off by more
than 8, and because the KV cache carries state forward the two rollouts drift
apart slowly (1.05 at block 6, 1.81 at block 11) the way any small numerical
perturbation does in an autoregressive model. Frame quality does not degrade
with it (luminance stddev 46.2 vs 46.2 at block 2, 65.5 at block 11), and
block 11 is visually clean.
It is committed separately from the bit-exact wins so it can be held back or
landed on its own. Reviewers should run the golden-replay gates
(`sd-abot-session --mode walkval --golden`) before treating it as settled;
those goldens were not available on the machine this was measured on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The scene-creation fix zeroes embedding rows past the last real token; note that in the doc, and document the new `scene pack: prompt rows N live / M` load-time log line (N == M warns that the padding was not zeroed).
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
1 participant
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.
Restores ABot-World generation quality (a regression that shipped through the 2026-08-11 line), makes the same failure loud, and cuts walk latency ~1.9×. Base:
2026-08-11.1. The quality regression (commits
0dfa679,70223bf)Introduced by the 2026-08-11 forward-port of causal world sessions. The reference
WanTextEncoderzeroes every prompt-embedding row past the last real token (u[v:] = 0); the July engine did this viazero_out_masked=true. The forward-port rewrote scene creation as a rawt5->compute(tokens, attention_mask)— but the attention mask only masks attention inside the encoder, it does not zero the output rows. So every pack written from the regressed engine carried live pad-token embeddings in all 512 context rows (prompt_embedsnorm 17 → 101, cosine 0.17 vs a healthy pack). The walk cross-attends the full 512-row context every block, the padding swamps the conditioning, and generation collapses into blur inside block 0. The first decoded frame still looked fine — it is a pure decode of bit-identical scene latents — which is what made it read as a walk bug rather than a scene-creation bug.Why CI missed it: the addon's ABot lanes were all structural (frame counts, dimensions, progress JSON, busy/teardown, "walk frames differ from idle"). Collapsed grey mush satisfies every one of them.
0dfa679— after the T5 encode insd_abot_scene_create, zero every output row whose additive tokenizer mask ≠ 0 (16 lines).70223bf— load-time diagnostic: logscene pack: prompt rows N live / 512, and warn when all rows are live (a pack the fix would have prevented). No behaviour change.2. Walk performance (commits
392588a,ce320f9,187ec98)RTX 5090, Vulkan, Q8_0 DiT, 832×480, KV cache, steady-state blocks:
392588ace320f9+187ec98Confirmed through the packaged addon: 1.27 s/block, 9.5 fps paced playback, zero stalls. 1.9× cumulative.
392588a— keep the compute buffer/allocator across the 5–6 graphs per block (the default freed and re-reserved multi-GB VRAM every graph; also cut cold start 12 s → 2.2 s); fusescale → add(mask) → soft_maxinto oneggml_soft_max_extover the graph's largest tensor.ce320f9— memoize the ~153 MB action planes within a block (invariant across its graphs); enable direct-conv decode for the small-3×3-conv taehv decoder (148 → 68 ms).187ec98— dropGGML_PREC_F32on the score matmul so a coopmat2 device routes it to the tensor-corematmul_f16_f16acc_cm2path instead of a scalar shader.Bit-exactness — please read before accepting
187ec980dfa679,70223bf,392588a,ce320f9are byte-identical to the previous build (mean |diff| 0.00).187ec98is not — it uses F16 score accumulation (block 2 differs by mean 0.29/255, ~0.2 % of pixels >8, drifting slowly with the KV cache the way any perturbation does in an autoregressive model; quality does not degrade — stddev 46.2 → 46.2 at block 2). It is a separate commit on purpose.Reviewer action: run the golden-replay gates (
sd-abot-session --mode walkval --golden) before accepting187ec98. Those goldens were not available on the measuring host. If they fail or the F16 accumulation is unwanted, drop187ec98alone — the other four are byte-identical and independent, and the fix + the bulk of the speedup (2.25 → ~1.5 s/block) land without it.Docs
docs/abot_world.mdnow documents the prompt-pad zeroing and the newprompt rows N live / Mlog line.Downstream
The addon PR (tetherto/qvac) builds against this branch through a temporary vcpkg overlay port and adds pack-conditioning + frame-quality CI guards that would have blocked the original regression. It should land after this merges and a registry port revision pins the fixed REF.