Repository navigation
Conversation
8bf7706 to
c85dfb9
Compare
d74489f to
9cf2886
Compare
5c9577a to
7862a94
Compare
9363c6e to
97ac124
Compare
055289d to
a0b094a
Compare
97ac124 to
abda7f8
Compare
1a734ea to
3a5e2db
Compare
027ec71 to
c700526
Compare
maocheng23
left a comment
There was a problem hiding this comment.
Reviewed the PR's own diff at c700526.
Checked locally (CPU, torch 2.14.1 / transformers 5.19.0):
- The preset and the README from_target mapping against the real Qwen/Qwen3.5-4B (config.json + safetensors headers):
- parse_draft with the target config passes;
- the metadata-only plan maps all 15 mtp.* tensors and binds both token tables (tie fallback);
- the native merge only adds is_causal, partial_rotary_factor and rope_theta.
- The preset's decoder layer against transformers' own Qwen3_5DecoderLayer, built from the real config with widths reduced, using distinct T/H/W positions: max diff 0.0.
- The interleaved layout matches HF and SGLang's apply_interleaved_rope.
- The contiguous path is bit-identical to the base over 1800 cases.
Two things I'd like fixed before the native recipe is advertised: the two inline comments.
Question (non-blocking): the native field checks in native.py only run at export. Would it be worth running the same merge as a preflight when from_target is set?
- The preset and the target's config.json are both available at train start.
- Today a preset that disagrees with the target trains to completion and fails only at export, or never, if the run isn't exported.
Options:
(a) keep it at export;
(b) factor the field/RoPE merge out of export_native into a pure function, and call it from canonical resolve_run when from_target is set, reading only the target's config.json and leaving the quantization rule to export (~80 lines).
I'd suggest (b).
| weights["model.language_model.embed_tokens.weight"] = torch.randn( | ||
| 64, 32 | ||
| ) | ||
| payload = convert_config(config) | { |
There was a problem hiding this comment.
This builds the target's config from convert_config(config), and the target's mtp.* names come from serving_weights (L82-87). So the round trip compares the preset with itself and can't catch a preset that disagrees with the real Qwen3.5-4B. The SGLang parity also builds its config from convert_config and feeds text positions.
I tried it at c700526:
- With the preset changed to
rope_theta: 1e6ormrope_interleaved: false, this file still passes, and the CUDA SGLang parity would too. - A wrong eps is caught, but only incidentally, by the new legacy-export block.
- Only a real
--format sglangexport against the target catches the other two, after training.
Two cheap checks would close this:
- Commit the real
config.json(3 KB) and the header entries of the 15mtp.*tensors + embedding (2.4 KB). Then runparse_draft(preset, target_config=real), plan the README mapping against a header-only store whosereadraises, and runexport_nativeagainst the real config. - On CPU, compare the preset's decoder layer with transformers' own
Qwen3_5DecoderLayerbuilt from the real config (widths reduced), with distinct T/H/W positions. It matches to 0.0 today and moves by 0.04-3 for a wrong theta, layout, norm offset or eps. This is the check design §7.3 asks for.
I have both written (two tests plus three small fixture files) if useful.
| and `training.strategy: mtp`: | ||
|
|
||
| ```yaml | ||
| draft: specforge/config/draft/presets/native/qwen3_5_4b_mtp.yaml |
There was a problem hiding this comment.
Following this recipe, feature capture asks SGLang for the last target layer. For a draft without target_layer_ids, capture_layers in training/canonical.py (from #1160) returns [num_hidden_layers - 1], i.e. 31 on Qwen3.5-4B, and the launch plan emits --return-aux-hidden-state-layer-ids 31.
SGLang's DFLASH hook taps layer k as the input of k+1:
Qwen3VLForConditionalGeneration.set_dflash_layers_to_captureadds 1;Qwen3_5ForCausalLMthen indexesself.layers[32]on a 32-layer model, which raises IndexError at server start.
Offline prepare requires the server's aux ids to match the contract, so it's affected too.
main avoided this on purpose: the legacy MTP provider uses _CAPTURE_LAYER_IDS = [1], with a comment. MTP only needs last_hidden, but SGLang returns that only when an aux layer is captured.
Suggest restoring [1] for final-hidden drafts in capture_layers, with the reason, plus a test that resolves this recipe against the real config and asserts every capture layer is <= L-2.
I read this from the pinned SGLang source (c757c4bb) rather than running a server. The resolved contract (31) and the emitted flags I did run, at c700526.
24d6d30 to
fae222a
Compare
c700526 to
a330b7b
Compare
0ca2f35 to
62434be
Compare
1fc9817 to
5c0a137
Compare
62434be to
b31bdcf
Compare
Add an ordinary YAML preset and explicit from_target recipe for the dense Qwen3.5-4B MTP head. Use the strict loader for owned weights and frozen token bindings. Make interleaved partial multi-axis RoPE explicit in configuration, execution and SGLang conversion while preserving legacy EAGLE section semantics.
Coverage: full preset geometry on meta, metadata-only rejection of incomplete/extra/quantized state, copy/export/copy equality and pinned-SGLang parity with reduced model widths. Native support is text-input, dense Qwen3.5-4B; no target-family model builder or preset registry is added. Existing local model validation used Torch 2.13.0+cu130 and Transformers 5.12.1; the repository's 5.19.0 Transformers pin was not exercised locally.
Base: #1162. Next: #1164.