Repository navigation
Conversation
Signed-off-by: Feathbow <feathbow@gmail.com>
Signed-off-by: Feathbow <feathbow@gmail.com>
Signed-off-by: Feathbow <feathbow@gmail.com>
Codex Review Summary: 🔄 RunningThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88bd157332
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| let mut conv_state = DeviceVec::zeros(ctx, g.qkv_width() * (g.conv_kernel - 1))?; | ||
| let mut qkv_conv = HiddenStates::zeros(ctx, g.qkv_width(), rows)?; | ||
| conv1d_prefill_batch_into( |
There was a problem hiding this comment.
Separate conv-state writes from prefill reads
When this layer prefills more than one token, conv1d_prefill_kernel reads conv_state for the first kernel_size - 1 positions while its t == seq_len - 1 threads overwrite the same state in the same launch (csrc/qwen35/conv1d.cu). CUDA blocks have no execution ordering, so the fresh zero history here can be replaced by tail tokens before early rows read it, producing nondeterministic or wrong qkv_conv; perform the state update in a later launch (or otherwise snapshot/synchronize it) before using this kernel for the new Qwen4 path.
Useful? React with 👍 / 👎.
| experts: g.experts, | ||
| top_k: g.top_k, |
There was a problem hiding this comment.
Align accepted MoE geometry with the router
With a smaller Qwen4 checkpoint, this copies the config-accepted 64-expert/top-4 geometry (explicitly accepted by config::tests::smaller_widths_still_pass) into Qwen4MoeGeometry, but the first route call's validate() only accepts 512 experts/top-10. Detection and loading therefore succeed while layer prefill always errors, defeating the intended smaller-model support; either reject these dimensions in Config or make routing support them.
Useful? React with 👍 / 👎.
| ensure!( | ||
| !used.is_empty(), | ||
| "no tensor under {prefix} in the checkpoint index" |
There was a problem hiding this comment.
Preserve the single-file checkpoint fallback
For a valid unsharded checkpoint containing model.safetensors, the shared load_shard_info returns the one path with an empty weight map so find_tensor can scan that shard. This check therefore rejects the empty used list before the file is mapped, making every single-file Qwen4 checkpoint fail with "no tensor under ... in the checkpoint index"; special-case the empty map by selecting the sole shard and relying on header/contract lookup.
Useful? React with 👍 / 👎.
Signed-off-by: Feathbow <feathbow@gmail.com>
… file checkpoints Signed-off-by: Feathbow <feathbow@gmail.com>
Signed-off-by: Feathbow <feathbow@gmail.com>
|
@codex check again please |
Description
Follows #1242.
pegainfer-qwen35intopegainfer_kernels::opsnext to the decode wrappers, the way feat(qwen4): share the Qwen3.5 GDN decode wrappers and prove their reuse #1159 moved those:conv1d_prefill_batch_into, the seven chunkwise stages behindgated_delta_rule_prefill_chunkwise_into, andGdrChunkwiseScratch35.pegainfer-qwen35keeps two small functions that size the scratch from its config. No kernel changes.pegainfer-qwen4gets a device half behind its ownqwen4feature.weights.rsloads one layer from the shards that hold it, checks that layer's slice of the tensor contract and uploads through the shared staged loader, which gains anexpert_stackrecord for the fused[experts, rows, cols]tensors.layer.rsruns onelinear_attentionlayer in prefill on the hyper-connection kernels, the GDN wrappers and the MoE block.moe.rsruns the block with the router kernel, a route metadata kernel that orders the picks by expert, one cuBLAS GEMM pair per expert over its slots (gemm_rows_span_at_into_checked), the fused SiLU, a weighted combine kernel, the shared branch and the block sum.tools/accuracy/dump_qwen4_layer_golden.pyrecords the reference layer on the checkpoint's own weights, once on the accelerator and once on the CPU, at 18 stage boundaries.layer::golden_gateholds the engine against it stage by stage, with the reference's own accelerator against CPU spread as the floor, and accepts a routing difference only where the reference's logits tie within two bf16 steps.launchstill refuses, the attention and PLE layers and the step come next, and the routed experts are the correctness first form, about a thousand launches and one host sync per layer at 2k tokens.Test Env
Single GPU (sm_89, x86_64) with CUDA 13.1. The layer reference was recorded with transformers 5.16.1 on the same card and, for the floor comparison, on a GH200.
Verification
cargo test -p pegainfer-qwen35 --features qwen35 --lib recurrent::cargo clippy -p pegainfer-kernels -p pegainfer-qwen35withqwen35, all targets, deny warningscargo clippy -p pegainfer-core -p pegainfer-kernels -p pegainfer-qwen4withqwen4, all targets, deny warningscargo clippy -p pegainfer-server --no-default-features --features qwen35,qwen4cargo test -p pegainfer-qwen4 --features qwen4 --libLayer 0 gate, max difference against the reference per stage, with the reference's own accelerator against CPU floor:
attn_outstream_after_attnmoe_outstream_outOne of the 32 tokens swaps one expert; the reference's logits for the two are one bf16 step apart, and the reference itself routes one token differently between the sm_89 and GH200 runs.
Not run: the engine side on GH200, and the GDN decode step on a carried state.