feat(models): dense Qwen3.5 custom attention#2943
Merged
Merged
Conversation
This was referenced Jul 3, 2026
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05690b5b6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
05690b5 to
72676ec
Compare
2fd0cf7 to
7d84eda
Compare
Implement dense Qwen3.5 (Qwen3_5ForCausalLM / Qwen3_5Model) on the PrimeRL attention modules, reusing the Qwen3.5-MoE building blocks (GatedDeltaNet, gated SDPA/flash attention, RMSNorm, rotary embedding) so ring/ulysses CP substitution patches the dense and MoE models consistently: - New models/qwen3_5 package: decoder layer dispatching on layer_types (linear_attention / full_attention), flash varlen cu_seqlens from position_ids, identity state-dict conversion (dense HF layout matches PrimeRL layout). - Shared normalize_qwen3_5_attn_implementation on the MoE module (eager→sdpa, kernels-community/vllm-flash-attn3→flash_attention_3), consumed by both models' attention factories and _check_and_adjust_attn_implementation. - Register Qwen3_5TextConfig / Qwen3_5ForCausalLM in AutoConfig and the custom causal-LM mapping; hook dense Qwen3_5GatedFlashAttention into substitute_ring_attn / substitute_ulysses_attn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The varlen kernel consumes a single packed row; batch rows beyond the first were silently dropped by the [0] indexing while cu_seqlens counted all rows' tokens. Fail loudly instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7d84eda to
e6b95f4
Compare
60df70d to
4fd7c5d
Compare
samsja
reviewed
Jul 7, 2026
mikasenghaas
reviewed
Jul 7, 2026
mikasenghaas
left a comment
Member
There was a problem hiding this comment.
no strong opinions here, looks fine to me
S1ro1
reviewed
Jul 7, 2026
S1ro1
reviewed
Jul 7, 2026
S1ro1
left a comment
Collaborator
There was a problem hiding this comment.
Left a comment, else lgtm
The custom attention path always receives one packed row, so avoid validating an invariant already guaranteed by the caller. Co-authored-by: Cursor <cursoragent@cursor.com>
samsja
approved these changes
Jul 9, 2026
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.
Summary
Part 2/7 of the split of #2485. Based on #2942 (
feat/sft-renderer-only) — only for stack ordering; the code depends onmainalone.Implements dense Qwen3.5 (
Qwen3_5ForCausalLM/Qwen3_5Model) on the PrimeRL attention modules, reusing the Qwen3.5-MoE building blocks (GatedDeltaNet, gated SDPA/flash attention, RMSNorm, rotary embedding) so ring/ulysses CP substitution patches dense and MoE consistently:models/qwen3_5package: decoder layers dispatch onlayer_types(linear_attention/full_attention), flash varlencu_seqlensfromposition_ids, identity state-dict conversion (dense HF layout matches PrimeRL layout).normalize_qwen3_5_attn_implementationon the MoE module (eager→sdpa,kernels-community/vllm-flash-attn3→flash_attention_3), consumed by both models' attention factories and_check_and_adjust_attn_implementation.Qwen3_5TextConfig/Qwen3_5ForCausalLMinAutoConfigand the custom causal-LM mapping; hooks denseQwen3_5GatedFlashAttentionintosubstitute_ring_attn/substitute_ulysses_attn.Engagement note: hub Qwen3.5 checkpoints ship composite (VLM) configs, and composite→custom dispatch (
_CUSTOM_VLM_MAPPING) lands in #2945 together with the state-dict remap that extracts the text tower — registering it here would crashget_modelon any composite checkpoint. Standalone, this PR's class engages for text-config checkpoints and is covered by the dispatch/state-dict unit tests; the e2e custom-vs-hf parity and perf comparison run at the #2945 tip (same tip, same data pipeline, onlymodel.impldiffers), where the attention code exercised is exactly this PR's.KL mismatch table
20 RL steps,
hendrycks-math, batch 64, Qwen3.5-0.8B,impl=custom, vLLM 0.24 (run):Bar is < 0.015; measured 50× under it, flat across all steps.
E2E (W&B project
pr2485)Loss parity + perf, custom vs hf at identical config/data/seed (8×H100, FA3, seq 8192, batch 64, cat packing; loaded class verified in logs). Run at the #2945 tip where composite→custom dispatch exists:
Parity to 1e-4 at both scales. The custom impl carries a consistent ~7–8% dense-text throughput cost (profiling follow-up; likely the gated-attention epilogue / varlen bookkeeping) — its value is capability: native packed-boundary consumption, CP hooks (+4% over the old sync path at cp=2, see #2946), MoE EP, and the VLM path, none of which the hf impl supports.
Review fix: Removed the redundant Qwen3.5 varlen batch-size guard because this path always receives one packed row. The other automated review findings are addressed by later PRs in the stack or unreachable — see the inline replies.
🤖 Generated with Claude Code
Note
Medium Risk
New training forward path for a flagship architecture (hybrid linear/full attention + custom flash varlen); mistakes would affect loss/CP correctness, but scope is additive with tests and shared MoE primitives.
Overview
Adds a dense Qwen3.5 custom path (
Qwen3_5ForCausalLM/Qwen3_5Model) wired intoAutoModelForCausalLMPrimeRLviaQwen3_5TextConfig, reusing MoE building blocks (GatedDeltaNet, gated SDPA/flash, RMSNorm, rotary) with dense MLP instead of experts.Decoder layers follow
layer_types(linear_attentionvsfull_attention). Flash paths derive varlencu_seqlensfromposition_idsfor packed training; KV cache is explicitly unsupported.Shared
normalize_qwen3_5_attn_implementationmapseager→sdpaand hub FA3 aliases→flash_attention_3, used by both dense and MoE factories plus_check_and_adjust_attn_implementation. DenseQwen3_5GatedFlashAttentionis hooked into ring and Ulysses CP substitution alongside MoE.State-dict conversion is identity for dense HF layouts. Unit tests cover HF key parity on meta, attention dispatch/normalization, and ring patching.
Reviewed by Cursor Bugbot for commit fd38616. Bugbot is set up for automated code reviews on this repo. Configure here.