[leo2ultimate]

Purpose

Batch supported H3 Qwen3-VL reference-frame inputs in one variable-length attention call per vision layer, including frames with different token lengths. Preserve the original per-frame SDPA path for unsupported inputs.

Changes

Before: a separate SDPA call for every frame (simplified; each frame's Q/K/V has shape [1, heads, tokens, head_dim]):

for q_frame, k_frame, v_frame in frame_qkv:
    outputs.append(F.scaled_dot_product_attention(
        q_frame, k_frame, v_frame,
        dropout_p=0.0, is_causal=False,
    ))

After: for supported CUDA inputs, one call over packed Q/K/V, with boundaries keeping frames independent:

from torch.nn.attention.varlen import varlen_attn

output = varlen_attn(
    query, key, value,
    cu_seqlens, cu_seqlens,
    max_seqlen, max_seqlen,
    scale=self.scaling, window_size=(-1, -1),
)

Frame boundaries are built once per vision forward and reused across layers. RoPE precision, scaling and token order are preserved.

The fast path reuses PyTorch's public can_use_flash_attention capability check, with the native varlen dtype/alignment constraints. CPU/FP32 and other ineligible inputs retain the original per-frame SDPA loop. Actual kernel errors and OOMs propagate; they are not caught and retried.

Test Plan

• TODO: verify correctness and performance improvement on GPU, including generated outputs, encoding/end-to-end latency and peak memory.

Local CPU checks:

PYTHONPATH=. python -m pytest \
  fastvideo/tests/encoders/test_minimax_h3_vision_attention_batching.py \
  fastvideo/tests/encoders/test_minimax_h3_qwen3_vl_vision.py \
  fastvideo/tests/contract/test_env_policy.py -q

Test Results

• 38 passed, 6 CUDA tests skipped
• Applicable changed-file pre-commit checks and git diff --check: passed. Full pre-commit run --all-files passed earlier in this PR.
• CPU benchmark smoke: original loop passed; native varlen comparison skipped.

CPU tests cover the real SDPA fallback and use a semantic substitute for CUDA varlen packing. Native GPU correctness, real-checkpoint numerical closeness, speedup and memory impact remain unverified. The existing official-parity test's zero tolerances are unchanged.

Checklist

• I ran pre-commit run --all-files and fixed all issues
• I added or updated tests for my changes
• I updated documentation if needed — benchmark usage is documented; no new configuration options.
• I considered GPU memory impact of my changes — GPU measurement is pending.

For model/pipeline changes, also check:

• I verified SSIM regression tests pass — pending GPU validation.
• I updated the support matrix if adding a new model — not applicable.

For new models or serving runtimes:

• I added or updated the native serving YAML and cookbook deployment, or explained why this is not applicable — no new model or serving runtime.
• I documented the GPU type/count and minimum VRAM with measurements, or marked the minimum as unknown — target-GPU memory requirements remain unmeasured.

[mergify]

Merge Protections

🔴 1 of 1 protections blocking · waiting on 👀 reviews and 🤖 CI

	Protection	Waiting on
🔴	PR merge requirements	👀 reviews and 🤖 CI

🔴 PR merge requirements

Waiting for

• #approved-reviews-by>=1
• check-success=fastcheck-passed
• check-success=full-suite-passed

This rule is failing.

• #approved-reviews-by>=1
• check-success=fastcheck-passed
• check-success=full-suite-passed
• check-success~=pre-commit
• title~=(?i)^\[(feat|feature|bugfix|fix|refactor|perf|ci|doc|docs|misc|chore|kernel|new.?model|skill|skills|infra)\]

[greptile-apps]

Confidence Score: 5/5

[Medium impact] No new blocking issue was found in the changes since the previous review.

Summary

Batches supported H3 vision frames through PyTorch varlen attention and keeps the original per-frame SDPA path for other inputs.

• Eligible CUDA reference frames share one attention call per vision layer.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Build packed frame boundaries once] --> B[Project Q/K/V and apply RoPE]
  B --> C{Flash checks pass?}
  C -->|Yes| D[One varlen call with frame boundaries]
  C -->|No| E[One SDPA call per frame]
  D --> F[Restore token shape and project output]
  E --> F

Loading

_{Reviews (3) · Last reviewed commit: "[bugfix] Preserve H3 vision SDPA compati..." · Reviewed by Greptile}

[leo2ultimate]

@fv-superagent review

Please review head 67a23ee54483f141c55646abdf3b01935a5a565b (base 866a3cdb91a85aef8561847365fc8564add9e4c8) for benchmarking readiness. The PR is now ready for review. If you find correctness, compatibility, CI or benchmark-method issues, please return those comments first. If it is ready and a compatible GPU/checkpoint are available, please run the bounded comparison below and report the results.

The change replaces the per-frame SDPA loop with one native PyTorch 2.12 varlen_attn call per vision layer. CPU/FP32 vision fallback was intentionally removed. Local evidence is 34 passed / 5 CUDA skipped; the earlier Greptile comment-only finding is resolved. No GPU performance or output-quality claim has been made.

Verified inputs

Reuse the repository's assets/images/couple.jpg (832×480 RGB) and assets/videos/robot_pouring.mp4 (1280×704, 16 fps, 93 frames, 5.8125 s, no audio). Generate these deterministic center-cropped/resized variants once and share the exact files between both versions:

ID	Transformation	Staged size / duration
I1	Couple image, center square crop	512×512 PNG
I2	Couple image, center 4:3 crop	640×480 PNG
I3	Couple image, center 3:4 crop	480×640 PNG
V1	First 3 s; crop x=14,y=0,w=1250,h=704	640×360, 24 fps, 72 frames
V2	First 2 s; crop x=288,y=0,w=704,h=704	384×384, 24 fps, 48 frames
V3	First 5 s; crop x=376,y=0,w=528,h=704	480×640, 24 fps, 120 frames

Images use Pillow ImageOps.fit(..., LANCZOS, centering=(0.5, 0.5)). Videos use crop → Lanczos scale → fps=24, no audio, libx264 CRF 18/yuv420p. Record your generated-file hashes.

Case A: I1 + V1. Case B: I1 + I2 + I3 + V1 + V2 + V3, in that order. B is a controlled stress case derived from the same two assets, not six independent scenes.

Actual CPU calls to H3's validate_references, prepare_reference and target-geometry helpers passed. H3 prepares images as 2048×2048, 2720×2048 and 2048×2720; videos as 1344×768, 768×768 and 768×1024. Video sampling yields 3/2/5 temporal blocks. Case B is within 9-image / 3-video / 12-total limits. Actual checkpoint processor grids/token counts and GPU memory fit still need verification.

Requested measurements

• Run the five existing native CUDA tests first (production 16 heads × 72).
• Compare the original code at the base revision with this PR using identical real weights, input files and settings, preferably separate processes. The committed benchmark only measures a single attention module with random weights; please do not describe that as full-encoder or end-to-end evidence.
• For A and B, measure the complete vision encoder: 2 warmups + 5 measured repeats, final/DeepStack feature parity, latency and starting/peak allocated/reserved memory.
• For A and B, measure complete generation: 1 warmup + 2 measured runs per version, if time permits. Use the same working OmniRef Spark checkpoint, BF16 vision, output 832×480 / 124 frames / 24 fps, seed 1234, guidance 1.0, and the checkpoint's eight-step contract. Preserve quantization, offload and DiT backend settings. Set FASTVIDEO_H3_REF2VA_MEMO_ENTRIES=0.
• Prompt: "A robot carefully pours a drink while two people watch nearby. Use the image references for the people's appearance and the video references for the pouring motion. Keep the camera steady."
• Compare uncompressed output frames and audio outside measured intervals; report similarity and visible differences. Capture actual processor grids and distinguish vision-only time from whole Qwen conditioning.
• Aim for about 60 minutes of GPU measurement after setup. Stop on parity/kernel failure or the first stress OOM. If time is insufficient, prioritize a complete A comparison and clearly mark B's generation pending. Do not change steps or other settings midway to improve a result.

Please use a benchmark-only runner if additional instrumentation is needed; if GPU, weights, the exact schedule, or a trustworthy encoder/end-to-end runner are unavailable, report those gaps first.

CI note: the prior fastcheck-passed failure references canceled build #2290; replacement #2292 is pending. GitHub pre-commit still shows the draft-era skipped run. Please inspect any actionable CI failures and identify checks requiring a fresh trigger or maintainer action.

[fv-superagent]

/test pre-commit

[fv-superagent]

fv-superagent score: 3/5

Tight diff, and the semantic CPU tests plus the honestly-scoped benchmark are good work. Two issues need an answer before this is merge-ready, both because the swap goes unconditional.

1. It breaks the contract in the H3 official-parity test for image/video. tests/local_tests/encoders/test_minimax_h3_qwen3_vl_parity.py:251 asserts atol=0.0, rtol=0.0 against the official Qwen3-VL stack for text, image and video. The official vision path splits frames and calls SDPA per chunk — exactly what the base loop did, which is why it was bit-exact. One packed varlen_attn over all frames is a different reduction and will not match. That test is MINIMAX_H3_RUN_ENCODER_PARITY=1-gated and outside the Encoder lane, so "34 passed / 5 skipped" does not cover it. The analogous DiT swap is opt-in for this exact reason (FASTVIDEO_MINIMAX_H3_FA4_PACKED_VARLEN, fastvideo/envs.py:359); this one has no opt-out.

2. The accepted fp32 text-encoder precision now hard-fails. --text-encoder-precisions accepts fp32 (fastvideo/configs/pipelines/base.py:183), which builds the vision tower in fp32; varlen_attn is fp16/bf16-only and has no CPU kernel, so an H3 ref2va run at fp32 — or any CPU vision forward — now raises where the base revision worked. If dropping fp32/CPU vision is intended, it needs an explicit guard and a doc line rather than a silent failure. (The default bf16 path is unaffected.)

CI. pre-commit was a draft-era skip that would not have re-run on its own, so I sent /test pre-commit; it passed. Greptile re-reviewed 67a23ee5 and found no new defect. buildkite/pr-fastcheck #2292 Encoder passed: 140 passed, 1 skipped (base was 127 passed, 1 skipped), so all five new CUDA tests ran on the DGX Spark GB10 and matched the per-frame SDPA loop within tolerance — the real kernel works on sm_121, which closes the "kernel untested" and contiguity concerns. Those tests use atol=8e-3, rtol=2e-2, which does not satisfy the atol=0/rtol=0 parity contract in item 1.

Repro commands, test-coverage note, and the requested benchmark

Parity repro: MINIMAX_H3_RUN_ENCODER_PARITY=1 pytest tests/local_tests/encoders/test_minimax_h3_qwen3_vl_parity.py -v -s

The committed tests/local_tests/encoders/benchmark_minimax_h3_vision_attention.py has no measurement bug: it reproduces the base loop with identical weights/inputs and pre-builds the packed metadata outside timing. Minor coverage note: the CPU test's non-contiguity assertion exercises the Linear input, not the strided q/k/v that .unbind(1) hands to the kernel — the CUDA tests now cover that in CI.

On the larger A/B you asked for: it needs a maintainer to clear a GPU run on a fork branch, so I left it; the five CUDA tests are already green in the Encoder lane. For that comparison, report a numeric feature diff together with the bf16 noise floor rather than a binary parity, and keep the two versions serialized on one GPU so timings don't overlap.

— fv-superagent, automated maintainer run by aryan5v

[leo2ultimate]

@fv-superagent review

Pushed 95c6c1c97cd4927778d690a608f8004b87f72cc8 to restore compatibility. The fast path now reuses PyTorch's public can_use_flash_attention check with the additional native-varlen dtype/alignment constraints; CPU/FP32 and other ineligible inputs use the original per-frame SDPA loop. Kernel errors and OOMs are still propagated rather than caught and retried.

Local checks: 38 passed, 6 CUDA skipped, applicable changed-file pre-commit checks and git diff --check passed. Tests exercise the real CPU FP32/BF16 fallback; native CUDA tests still need execution. The benchmark reports a skipped varlen comparison when the compatibility path runs, rather than counting it as a varlen result.

On your exact-parity concern: we have not changed the official-parity test's zero tolerances, and we are not claiming that real-checkpoint GPU outputs already pass allclose. Our proposal is to keep text-only parity exact and consider dtype-appropriate, nonzero torch.testing.assert_close tolerances for the optimized image/video path, backed by measured feature differences and generated-output preservation.

Could you run or inspect the gated official-parity test on the base and this head with identical weights/inputs, and report whether exact equality actually fails, plus max/mean absolute differences and the BF16 numerical noise floor? If the differences are consistent with expected kernel rounding, would tolerance-based image/video parity be acceptable, or is bit-exact official parity a required contract here? We should establish that before changing any threshold.

Please return any remaining correctness/readiness concerns first. If the real-checkpoint GPU comparison requires maintainer clearance, identify that requirement rather than treating CPU tests as GPU evidence.

[fv-superagent]

fv-superagent score: 4/5 (was 3/5)

Re-reviewed the delta from 67a23ee5 to 95c6c1c9.

Resolved — fp32/CPU compatibility. _can_use_vision_varlen now gates the packed call on ROCm exclusion, fp16/bf16 dtype, head_dim % 8, and PyTorch's can_use_flash_attention; everything else takes the original per-frame SDPA loop. The new tests exercise the real fallback on CPU fp32/bf16 and CUDA fp32 with atol=0 against the legacy loop, and the benchmark reports a skip instead of counting a fallback as a varlen result. That closes my earlier item 2.

Still open — exact image/video parity is a contract decision, not a code fix. On CUDA bf16 the fast path is eligible, so tests/local_tests/encoders/test_minimax_h3_qwen3_vl_parity.py:251 cannot stay at atol=0/rtol=0 for image/video; text-only is untouched. Your proposal — keep text exact, use a dtype-appropriate tolerance for the optimized image/video path, backed by measured feature differences — is reasonable, but it changes a repo contract, so a maintainer owns it. The earlier request-changes stands until that is settled.

On running the gated parity test. It needs CUDA bf16, the official text_encoder/ checkpoint and the Transformers stack, and this is a fork branch: the GPU runner requires a maintainer's /agent gpu-ok <full sha> comment before it will fetch fork code. I have a base-SHA job queued to validate the harness and establish the noise floor, and the head run is waiting on that marker. I'll report the measured max/mean differences once both land; I am not treating the CPU tests as GPU evidence.

CI. Greptile re-reviewed 95c6c1c9 and found no new issue. pre-commit passed on the push. buildkite/pr-fastcheck #2296 is queued behind an agent backlog; the previous head's Encoder lane was green (140 passed, 1 skipped).

— fv-superagent · CPU is back on the guest list · run by aryan5v