frost(sdpa): derive THD token capacity from the view's element span (fixes #613) - #706
frost(sdpa): derive THD token capacity from the view's element span (fixes #613)#706vedaanta wants to merge 2 commits into
Conversation
…VIDIA#613) The zero-host-read THD execute (NVIDIA#606/NVIDIA#608) derives the packed token extents host-side as numel() // token_stride. That is wrong on both edges for the buffers real integrations bind: - A non-packed VIEW — a K/V slice of a kv-interleaved [T, 2, H, D] record, the layout torch.nn.attention.varlen users produce by slicing a fused KV projection — holds T tokens but only T*H*D of the record's elements, so the derived extent HALVES and the TMA descriptors cut off half the tokens: silently wrong O on every such call (issue NVIDIA#613; also 40 upstream PyTorch test_varlen_attention failures through the python-API integration). - Deriving from the untyped storage instead over-claims into ALLOCATOR SLACK, which is not benign: rows between the real packed total and the extent are masked but still multiplied (P == 0 times V), so they must be FINITE — TMA zero-fill only covers rows at or beyond the extent. A slack row carrying NaN bit patterns poisons whole sequences through 0 * NaN. Fix: capacity = the largest T whose final token's ROW still fits in the buffer's own element SPAN (1 + sum((size-1)*stride)). The span is exact on both edges: flat capacity buffers give exactly their token capacity (no slack), and interleaved/gapped views give exactly T. Every row below the capacity lies in caller-provided finite elements; every row at or beyond it TMA-clips to zeros. One shared helper serves the SM100 f16 path and the SM120/FP8 _cap sites. Verified on SM100 (isolated env): the NVIDIA#613 kv-interleave repro 41% -> 0 mismatches (frost-served); test_sdpa_random_fwd_ragged_L0 5-seed slice 84/84 (no regressions); fp8 THD ragged slice green; the new deterministic regression test (fused-record K/V views vs packed binding, torch.equal) fails on develop and passes with the fix; upstream PyTorch test_varlen_attention returns from 100 pass / 69 fail to its 140 / 29 impl-identity baseline with the torch-ops stack applied on top. Fixes NVIDIA#613. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe change adds addressable THD capacity calculation for SM100 and SM120. THD packing uses it for Q/O and K/V extents. Ragged strides use derived generation. A regression test validates interleaved strided K/V views. ChangesTHD capacity correction
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change corrects THD token-capacity handling for packed and interleaved views, with the supplied regression and compatibility checks passing; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The seeded per-tensor token-gap draw (NVIDIA#516) lives in ExecConfig.fill_derived_fields and only fills strides left None — but RandomizationContext, which drives every test_sdpa_random_*_ragged sweep, explicitly assigned packed bshd strides in its ragged branch. Net effect: the randomized ragged fleet has NEVER bound a non-packed THD stride, and for packed buffers the numel()//token_stride capacity heuristic is exact — which is precisely why these sweeps stayed green while issue NVIDIA#613 (interleaved K/V views halving the TMA extent) shipped and had to be found through an external integration. Fix: the ragged branch leaves Q/K/V/O strides None and __call__ ends with fill_derived_fields() — one source of truth for the gap draw and its auto-packed fallbacks (cu / offset-multiplier forms NVIDIA#538, 1-byte dtypes NVIDIA#537). The head_major stats stride and the whole dense branch are untouched. Census over the fwd ragged L0 slice (84 configs): before, 0/84 drew a gap although each config's own rng_geom_seed hand-draws nonzero gaps; after, 84/84 draw gaps and ALL 84 would have failed under the old capacity formula. Verified on SM100 (cuDNN 9.26.0.33, CUDNN_FRONTEND_ENABLE_FROST_ENGINES=1): with the NVIDIA#613 fix the gapped fwd ragged L0 slice passes 84/84 (all frost-served) — with the pre-fix adapter swapped in it fails 80/84, i.e. this wiring alone would have caught NVIDIA#613 the day the heuristic merged. bwd ragged L0 slice 158/158, identical to the unwired control on the same lib (the backend serves every gapped gradient combination); ragged_unified_L1 24/24 and offset_multiplier_unified_L1 24/24 (cu / mult forms stay packed via the existing fallbacks — 20/20 each in the offline census); the stride-override unit test still passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Added bd7a8f2 — The coverage seam. The seeded token-gap draw (#516) lives in The commit makes the ragged branch leave Q/K/V/O strides Red-team evidence (SM100, cuDNN 9.26.0.33,
i.e. this wiring alone would have caught #613 the day the capacity heuristic merged. Post-fix matrix, all green:
Note: the bwd baseline requires a backend with the dSink+ragged-stats fixes (9.26 here); on a 9.24 lib the bwd ragged sweep fails for that unrelated pre-existing backend reason with or without this commit. |
|
Run CI. |
|
Follow-up filed as #718. While reasoning about how much of the capacity contract this PR actually buys, I confirmed the other half is still open: Nothing here changes this PR — the span fix is still needed and still correct, since the clamp only helps tensors that get one. Flagging the linkage so the two are reviewed together. |
Problem (issue #613)
The zero-host-read THD execute (#606/#608) derives the packed token extents host-side as
numel() // token_stride. That is wrong on both edges for the buffers real integrations bind:[T, 2, H, D]record — the layouttorch.nn.attention.varlenusers produce by slicing a fused KV projection — holdsTtokens but onlyT*H*Dof the record's elements, so the derived extent halves and the TMA descriptors cut off half the tokens: silently wrong O (41% mismatches on the frost(sdpa): THD zero-host-read extents break non-packed views; unit decode is extent-sensitive (regression from #606, also in #608) #613 repro). Through the PyTorch python-API integration this now breaks 40 upstreamtest_varlen_attentioncases, because the engines recently started claiming tiny-window THD configs they previously declined.P(=0) × V), so they must be finite; TMA zero-fill only covers rows at or beyond the extent. One slack row carrying NaN bit patterns poisons whole sequences through0 × NaN(reproduced: batch-wide NaNs in the ragged sweeps).Fix
Capacity = the largest
Twhose final token's row still fits in the buffer's own element span (1 + Σ (size−1)·stride). The span is exact on both edges:T(the last token needs only its own row footprint, not a full record span).Every row below the capacity lies in caller-provided finite elements; every row at or beyond it TMA-clips to zeros. One shared
_thd_capacityhelper serves the SM100 f16 sites and the SM120/FP8_capsites (packed contract included).Verification (SM100, isolated worktree + venv, no shared JIT cache)
test_repro, seeded)torch.equal)test_sdpa_random_fwd_ragged_L05-seed slice (84 tests)test_varlen_attention(with the torch-ops stack on top)The regression test lives next to the #606 suite (
test_dsl_sm100_thd_interleaved_kv_views) and encodes both failure modes in its docstring.Fixes #613.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests