Repository navigation
[Draft design 06/21] Rotary components and attention execution - #1124
Conversation
7b03644 to
3d3db4d
Compare
71c911e to
b4838ef
Compare
dbfb11d to
7dd574b
Compare
7dd574b to
e83a0b4
Compare
af80161 to
f4613a3
Compare
eae230d to
cca68f7
Compare
maocheng23
left a comment
There was a problem hiding this comment.
Inline notes below. Checked on CPU at the repo pins (torch 2.14.1, transformers 5.19.0): across the 9 changed test modules, two tests fail. They are test_draft_gqa.py (fp32 MHA, layout) and test_draft_rotary.py (device=). With the two fixes suggested inline, all 43 CPU tests pass. The GPU PR Test workflow only runs for PRs based on main, so CUDA precision and gradients haven't been exercised for this branch.
23f98c3 to
eab7f73
Compare
5fefe58 to
5d3494a
Compare
2ce3324 to
04d350b
Compare
maocheng23
left a comment
There was a problem hiding this comment.
Thanks. The follow-ups address nearly everything from the first two rounds; I re-checked each item at e8d556c:
- Layout: a description (tensor,
mask_modorPaddingMask) plusactive_queries. - Backends:
prepare/attend(Flex, SDPA, FA2), with the mask built once per forward and the LSE exposed. - Errors: the eager backend is bool-only; FP32 RoPE tables are enforced; the
apply_rotarylength checks are in. - Layout batches broadcast, and the registry test is isolated.
- RoPE semantics match SGLang: the
interleavedconvention lives on the RoPE node; linear scales positions; dynamic uses the fixedfactor × originallength; tables are cached per device. - Stack hygiene: ordinary commits.
Verified on CPU at the repo pins:
- The changed tests pass: 184 passed, 12 skipped (CUDA/SGLang/flash).
- Attention and masks are still bit-equal to main's DFlash eager path when RoPE is factored out (identity tables, fp32 and bf16).
- The callable
mask_modpath matches main's DFlash mask. - Linear and dynamic RoPE match SGLang's formulas.
- Rotation arithmetic matches main's compiled EAGLE path, given the same tables.
Remaining:
- The RoPE numerics change is now explicit and tested. I'm fine taking it as an intended deviation, since it follows SGLang serving and main's bf16-rounded frequencies are the bug. Let's record it in the design doc (#1076 §10 says steps 1–5 don't change numerics) and in the usage change list.
- CI. Flex, FA2, the SGLang references and the compiled-legacy rounding test only run on CUDA, and the GPU
PR Testworkflow doesn't run for stacked PRs. Could you run it viaworkflow_dispatchon this branch at the repo pins (torch 2.14.1 / transformers 5.19.0) and note the head in the body? - Refresh onto #1116's current head. The merge is textually clean, but #1116's newest commit currently breaks on transformers 5.19 (
_initialize_weights()getsis_remote_code), which will also failtest_meta_materialization_uses_generic_initializationhere. - Serving boundary for the new qk-norm options. SGLang's DFlash and EAGLE drafts hard-code a per-head, weighted qk RMSNorm before RoPE.
qk_norm_position="after_rope",qk_norm_scope="projection"andhas_weight=Falseare accepted for every architecture, so such a draft trains but can't be served faithfully. The SGLang converter (or the architecture constraints) should reject them for those architectures.
|
Non-blocking nits:
|
The backbone memo admitted any RotaryEmbedding subclass by node and head_dim, so a custom subclass with per-module state silently received another layer's cosine/sine tables. Only the exact built-in types are determined by their node; any other module now shares only with itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Under autocast, weighted QK RMSNorm promotes Q/K to FP32 and the shared attention boundary casts V to match, so FlashAttention rejected inputs that eager and SDPA accept. Cast them to the autocast dtype, as SDPA and the legacy HF FlashAttention path do. Without autocast, FP32 inputs are still rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
flash_attn's varlen backward ignores the gradient of its LSE output without marking it non-differentiable, so a loss that combines outputs through LSE would train with silently wrong Q/K gradients. The eager and Flex LSEs are differentiable. Raise during backward until a caller needs the gradient; losses on the attention output are unaffected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`q - window` and `q + window` overflow when the window approaches the position dtype's maximum, which silently emptied the mask for every query but the first. Comparing the query-key distance against plus and minus the window restores the earlier `abs(q - k) < window` semantics without that edge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SGLang keeps an FP32 cosine/sine cache for its base rotary embedding on CUDA, which canonical RoPE matches, but its multi-axis rotary embedding casts the cache to the activation dtype before rotating. State that multi-axis drafts differ from serving by that rounding instead of claiming one convention. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`torch.is_autocast_enabled` raises for meta tensors, so FP32 meta inputs got a RuntimeError instead of the backend's named ValueError, and CPU inputs were cast before being rejected. Only CUDA inputs reach FlashAttention, so query CUDA autocast only for them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A Python window larger than the position dtype's maximum wraps in tensor comparisons, so an int32 layout with a 2**31 window silently lost every key. No distance between such positions can reach that window, so the layout resolves it to no window once, for the dense, Flex and FlashAttention paths. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SGLang keeps its base rotary cache in FP32 on CUDA only for head sizes 64, 128, 256 and 512; other sizes take a fallback kernel that casts the cache to the query dtype, like the multi-axis embedding does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
maocheng23
left a comment
There was a problem hiding this comment.
Approving. I re-checked at a5396f9:
- N9, N14, N15, N17 and N18 are fixed.
- The numerics decision is recorded here and in #1076.
- PR Test passed with every CUDA, FA and SGLang test running.
I pushed eight small follow-ups on top (d754c2ad..bf824a4d), each with a test:
- The backbone RoPE memo shares tables across modules only for the built-in rotary types. Before this, a custom subclass with per-module state silently got another layer's tables.
- FA2 casts FP32 Q/K/V to the autocast dtype when autocast is on, like SDPA and HF. Weighted qk-norm promotes them to FP32 under autocast, so FA2 used to reject them. It checks for CUDA first, so meta and CPU inputs still get the named error.
- FA2's LSE raises on backward, because flash_attn drops that gradient.
- The window rule compares distances. A window at or beyond the position dtype's limit no longer overflows or wraps; the layout resolves it once for dense, Flex and FA2. The shifted form came from my earlier nit, sorry.
- The docs say where canonical RoPE matches SGLang: the FP32 base cache for head sizes 64/128/256/512. Other sizes and multi-axis RoPE are rounded to the activation dtype in SGLang.
Follow-ups outside this PR:
Brings in #1124's last commits after a5396f9: rotary tables shared only across built-in rules, FlashAttention autocast casting, device check and LSE gradient rejection, distance-based windows that ignore oversized widths, and docs. No conflicts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Implement registered rotary components and GQA/MHA projections with shared attention preparation and execution. Eager, SDPA, Flex and FlashAttention-2 implement the same prepare/attend interface; masks and rotary tables are reused across equivalent layers within each forward.
RoPE follows SGLang serving semantics: linear scaling divides positions, dynamic NTK uses a fixed factor × original-context length, and frequencies/tables remain FP32. Rotation computes in FP32 and casts once to the Q/K dtype. Static frequencies, including YaRN's ramp, are cached per device and invalidated on module moves/casts. Dynamic NTK has no position-dependent host sync.
This is an intentional numerical change from legacy DFlash's bf16-cast frequencies and eager per-operation rounding, and from EAGLE's rounded tables/history-dependent dynamic cache. Tests compare the legacy production rotation functions at 32k/65k positions and record the difference. SGLang table/native-reference comparisons use exact equality; the CUDA kernel comparison allows bf16 kernel rounding. Legacy EAGLE dynamic configurations remain explicitly unsupported.
Rotation pairing is explicit on the shared RoPE node. GQA takes a resolved RopeSpec and supports QK normalization before/after RoPE and per head/across the full projection, including weightless RMSNorm as used by SGLang Llama4. The legacy codec resolves old conventions explicitly; canonical schemas and golden fixtures require all structural fields. Input validation is shared by projection implementations.
Validation at
fc2aff66af6538fc360773338d2ae7202f534ef6on torch 2.14.1+cu130 and transformers 5.19.0:Latest review fixes align backend names with training (
eager,sdpa,flex_attention,fa), reject unsupported canonical USP, use explicit Flex kernels, validate padding across backends, share head-independent masks, and avoid tensor-ID Dynamo guards. Multi-axis interleaving, meta rotary execution, legacy field rejection and per-head inactive rows have regression tests. Canonical schema v1 remains provisional until the format cutover.The branch includes #1116's
93c754bcinitialization fixes through an ordinary merge. The RoPE numerics/schema wording is updated in #1076 (8fefec6b); QK-norm serving restrictions belong to #1162 and are implemented there (e7199330, two focused export tests passed).The resolved-RoPE constructor, shared validation, boolean causal comparison and SDPA backend now belong to this PR. The current implementation and follow-ups are ordinary commits rather than implementation hidden in merge commits. Downstream consumers still need to be synchronized when the PR stack is refreshed.
Base: main (#1116 is merged). Next: #1130.
Latest main refresh:
c63bd6a33c63145a1ac4540db5b3c8a35752a80fmerges main132d35d3, preserves the EAGLE regression case, and resolves the PR conflict. Post-merge CPU config/model/MTP validation: 411 passed, 17 skipped (428 total); full-repository pre-commit passed. The workflow_dispatch above tests the earlierfc2aff66head, not this merge commit.