Skip to content

feat(models): dense Qwen3.5 custom attention#2984

Merged
samsja merged 1 commit into
mainfrom
feat/qwen35-dense-attention-main
Jul 10, 2026
Merged

feat(models): dense Qwen3.5 custom attention#2984
samsja merged 1 commit into
mainfrom
feat/qwen35-dense-attention-main

Conversation

@hubert-marek

Copy link
Copy Markdown
Contributor

Summary

  • add a dense custom Qwen3_5ForCausalLM implementation using PrimeRL attention modules
  • register dense Qwen3.5 model/config dispatch and context-parallel attention substitutions
  • add unit coverage for dispatch, state-dict conversion, and attention selection

Validation

  • uv run --no-sync ruff check ... — passed
  • uv run --no-sync pytest tests/unit/train/models/test_qwen3_5.py -q --tb=short — blocked during collection because flash_attn is not installed in this environment

The custom implementation previously matched the HF path to 1e-4 loss at 0.8B and 9B in the linked stacked-PR runs, with mismatch KL 0.0003 across 20 RL steps.

Made with Cursor

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>
@hubert-marek hubert-marek marked this pull request as ready for review July 9, 2026 21:07
@hubert-marek hubert-marek requested a review from samsja July 9, 2026 21:08

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0637368. Configure here.

return BaseModelOutputWithPast(last_hidden_state=hidden_states)


class Qwen3_5ForCausalLM(Qwen3_5PreTrainedModel, GenerationMixin):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing KL validation table

Medium Severity

This PR adds a new custom Qwen3_5ForCausalLM, but the description does not include the required mean KL mismatch table (20 steps, math environment, batch_size=64, all entries below 0.015). Narrative KL figures are not a substitute for that table.

Fix in Cursor Fix in Web

Triggered by project rule: BugBot Instructions

Reviewed by Cursor Bugbot for commit 0637368. Configure here.

torch._dynamo.mark_dynamic(cu_seqlens, 0)
else:
cu_seqlens = None
max_seqlen = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3D positions break cu_seqlens

Medium Severity

When flash attention is enabled, Qwen3_5Model builds cu_seqlens via get_cu_seqlens_from_position_ids on raw position_ids. For 3D MRoPE inputs, flattening yields wrong segment boundaries, unlike the MoE model’s explicit 3D handling.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0637368. Configure here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0637368614

ℹ️ 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".

_CUSTOM_CAUSAL_LM_MAPPING.register(MiniMaxM2Config, MiniMaxM2ForCausalLM, exist_ok=True)
_CUSTOM_CAUSAL_LM_MAPPING.register(NemotronHConfig, NemotronHForCausalLM, exist_ok=True)
_CUSTOM_CAUSAL_LM_MAPPING.register(Qwen3MoeConfig, Qwen3MoeForCausalLM, exist_ok=True)
_CUSTOM_CAUSAL_LM_MAPPING.register(Qwen3_5TextConfig, Qwen3_5ForCausalLM, exist_ok=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Route composite Qwen3.5 configs to the custom model

Registering only Qwen3_5TextConfig leaves the shipped Qwen/Qwen3.5-* configs, whose top-level config is model_type='qwen3_5' with a nested text_config, outside the custom path. get_model() classifies those configs as VLMs and consults _CUSTOM_VLM_MAPPING, which still has no qwen3_5 entry, so impl='auto' selects the HF implementation and even impl='custom' goes through AutoModelForImageTextToText instead of this new PrimeRL attention implementation. This means the new dense Qwen3.5 custom model is not used for the primary Qwen3.5 checkpoints unless the checkpoint has been rewritten as a qwen3_5_text config.

Useful? React with 👍 / 👎.

Comment on lines +213 to +215
else:
cu_seqlens = None
max_seqlen = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve packed boundaries on non-flash attention

When dense Qwen3.5 is run with attn='sdpa'/'eager' on normal packed RL batches, this branch drops the boundary information encoded by reset position_ids. The model still has linear_attention layers, and Qwen3_5MoeGatedDeltaNet only resets its causal conv/DeltaNet state when cu_seqlens is supplied, so packed examples can leak state into each other; the SDPA full-attention layers also get no mask to prevent cross-example attention. Please either compute/use packed boundaries for this path or reject packed non-flash Qwen3.5 runs.

Useful? React with 👍 / 👎.

@hubert-marek

Copy link
Copy Markdown
Contributor Author

Superseded by #2943; keeping the existing stacked PR and rebasing it onto main after its prerequisite merges.

@samsja samsja merged commit 9b47b77 into main Jul 10, 2026
14 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants