feat(dflash): remap fused QKV weights for Laguna warm-start - #922
Conversation
…rm-start Laguna-style DFlash checkpoints use fused qkv_proj + per-head g_proj gating, while DFlashDraftModel expects separate q/k/v_proj (Qwen3-style). Add a _remap_weights step that splits fused QKV, drops incompatible keys (g_proj, aux_hidden_norms), and slices fc.weight when the source has more target layers than the model expects. Enables warm-starting DSpark training from published Laguna DFlash checkpoints. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Orestis Zambounis <orestis-z@users.noreply.github.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesDFlash weight conversion
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 Require approval from approved reviewers listAll pull requests must have at least one approving review from a member of the approved reviewers list before merging.
|
|
The quality checks have failed. Please run |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/speculators/convert/dflash/converter.py`:
- Around line 169-173: Reject mixed fused and separate projection formats in the
remapping logic around the qkv_proj handling: before inserting any q_proj,
k_proj, or v_proj destination, check whether that destination already exists in
remapped and raise ValueError on a collision. Add a mixed-format checkpoint test
in tests/unit/convert/test_dflash_converter.py covering the q_proj/k_proj/v_proj
destinations and asserting ValueError.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e6f9da45-7fbd-498b-8579-3129bc204862
📒 Files selected for processing (2)
src/speculators/convert/dflash/converter.pytests/unit/convert/test_dflash_converter.py
shanjiaz
left a comment
There was a problem hiding this comment.
Looks good pending quality!
Laguna models use nested per-layer-type rope_parameters (with `full_attention` and `sliding_attention` sub-dicts), which Qwen3RotaryEmbedding does not understand — it expects a flat dict with `rope_theta` at the top level. Since all DFlash draft layers use sliding-window attention, flatten to the `sliding_attention` variant before constructing the rotary embedding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Orestis Zambounis <orestis-z@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Orestis Zambounis <orestis-z@users.noreply.github.com>
…n_state_layer_ids When converting a z-lab DFlash checkpoint without explicit aux_hidden_state_layer_ids, the converter auto-derives them from dflash_config.target_layer_ids with a +1 offset. This included the last verifier layer (e.g. layer 48 for a 48-layer model), but speculators treats that layer separately as verifier_last_hidden_states — it should not appear in aux_hidden_state_layer_ids. Including it caused the fc layer to be sized for one extra feature, leading to dimension mismatches at inference when vLLM builds the DFlash/DSpark model from the converted checkpoint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Orestis Zambounis <orestis-z@users.noreply.github.com>
speculatorsbot
left a comment
There was a problem hiding this comment.
The weight remapping logic in _remap_weights is clean and well-tested — the QKV split, key dropping, and fc slicing all make sense for the Laguna warm-start use case. The _build_config change to exclude the last verifier layer from aux_hidden_state_layer_ids is correct and pairs well with the fc slicing.
One concern about the rope_parameters fix in core.py — see inline comment.
🤖 Generated with Claude Code using the /pr-review skill
…warning Reject checkpoints that contain both fused qkv_proj and separate q/k/v_proj keys, and warn when flattening nested rope_parameters to sliding_attention while full-attention layers are present. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Speculators Bot <23146389+orestis-z@users.noreply.github.com>
Summary
_remap_weights()toDFlashConverterthat splits Laguna-style fusedqkv_projinto separateq_proj/k_proj/v_proj, drops incompatible keys (g_proj,aux_hidden_norms), and slicesfc.weightwhen source has more target layers than needed.poolside/Laguna-S-2.1-DFlash) instead of training from scratch (~13.5 day ETA → significantly faster convergence).Test plan
poolside/Laguna-S-2.1-DFlashsucceeds (6 fused qkv_proj split, 12 keys dropped, fc sliced 18432→15360)🤖 Generated with Claude Code