fix(ds4): keep ROCmFPX generation byte-safe#494
Conversation
davide221
changed the title
|
Updated this PR with a DS4 correctness fix from the lucebox2 antirez baseline check.\n\nFindings on lucebox2 / Strix Halo with antirez/deepseek-v4-gguf:\n- PR490/ac06 and pre-fix #494 both loaded all 256 experts/layer and then fell into the legacy non-HC full graph path. That path produced deterministic garbage (e.g. repeated '哈哈' / 'Giya') while reporting ~16.7 tok/s.\n- Forcing partial-hot hybrid via DFLASH_EXPERT_BUDGET_MB=50000 proved the HC-enabled path is coherent, but only ~3.4-4.5 tok/s because it leaves many experts cold.\n- This update keeps the all-hot managed allocation but routes non-hybrid DS4 execution through deepseek4_step_layer_range(), which includes HC, and resets DS4 cache counters/buffer per fresh request.\n\nVerification after the patch:\n- Built on lucebox2: cmake --build build-hip-antirez -j 8 --target dflash_server\n- Ran DeepSeek-V4-Flash-IQ2XXS-w2Q2K-AProjQ8-SExpQ8-OutQ8-chat-v2.gguf with DFLASH_DS4_TIMING=1\n- Three independent chat requests in one server process completed without crash. Outputs were coherent (math/memory/capital prompts), though still somewhat repetitive.\n- Decode timing was stable at ~11.2-11.3 tok/s with all experts in the managed buffer and HC enabled.\n\nSo the apparent ~16-17 tok/s result was not valid quality-wise; the current correct all-hot baseline is ~11.3 tok/s. Next optimization work should target HC-pre cost (~1.1s / 63 decode steps), attention (~2.18s), and FFN compute (~1.42s) in the HC layer-range path, rather than the legacy full graph path. |
|
ROCmFPX quality fix update:
This means the previous ~16 tok/s number is not the current coherent all-fixes baseline. The next perf task is restoring direct GPU HC for quantized ROCmFPX HC weights, likely by pre-dequantizing the small HC |
|
Follow-up on Weicj PR489 speed claim: I built exact Findings:
Conclusion: 16 tok/s is reproducible only on the HC-less/incorrect direct path. To get 16+ with correct generation, we need to bring HC semantics into the fast full-step/direct path or fuse the current layer-range HC path to remove host round-trips and per-layer graph overhead. |
|
Updated with What changed:
lucebox2 / Strix Halo validation, Antirez GGUF,
This is a small correctness-preserving improvement over the previous ~11.3-11.4 tok/s path, not the 16 tok/s target yet. The remaining large costs are still attention, FFN, and HC-pre graph/kernel work. |
davide221
force-pushed
the
codex/ds4-rocmfpx-optimizations
branch
from
be8cf4b to
5dc41d6
Compare
davide221
changed the title
|
Status note: after rebuilding this PR on top of current main, the unsafe ROCmFPX/HC fast-path experiments have been removed. The target state is correctness-first: generation follows the main DS4 EOS handling and this PR only keeps the byte-safe compressor dependency cleanup plus the default DS4 system prefix for the ROCmFPX Src GGUF. On lucebox2 / Strix Halo, the validated fixed ROCmFPX runtime is in the ~15 tok/s range with correct generation:
So: main + this PR should be treated as the correct ~15 tok/s baseline. The earlier 22-25 tok/s HC/fused experiments are intentionally not in this PR because they were not byte-identical and could produce divergent/bad generation. |
davide221
changed the title
The previous pin lacked ROCMFPX (type 101) support, so the DS4-Flash ROCMFPX GGUF failed to load from a clean checkout of this branch.
deepseek4_step's non-hybrid branch skips hyper-connections entirely and generates garbage on all-hot boxes (Strix). Dispatch to deepseek4_step_layer_range when no MoE hybrid engine is present.
… fused decode Byte-identical, default ON (per-row accumulation order unchanged): - Ds4HcMatvecPool: persistent spin pool for HC fn matvecs and hc_post - f16c dot kernels with scalar-order adds (bit-exact vs scalar) - Ds4DecodeSharedInputs: per-step scalar inputs uploaded in 2 tensor_sets instead of ~430 tiny ones Opt-in, default OFF (reorder fp math, deterministic but not bit-identical to the reference path): - DFLASH_DS4_FFN_RAW_MMID=1 / DFLASH_DS4_FFN_FUSED_COMBINE=1: expert-FFN accumulation reordering - DFLASH_DS4_ROCMFPX_HC_GPU=1: GPU HC pre-mix - DFLASH_DS4_FUSED_DECODE=1: single-graph decode with GGML_OP_DS4_HC GPU hyper-connections, padded compressed-KV spans and an additive row mask (-1e30 underflows to exactly 0 in softmax) so the graph topology is stable across steps Steady-state decode, DS4-Flash ROCMFP2 on Strix gfx1151 (pinned iGPU perf level): 14.2 -> 17.6-18.5 tok/s on the byte-identical default path; ~19 tok/s with fused decode. Requires the iGPU perf-level pin for stable numbers (sclk sags 2900->1473 MHz under sustained decode otherwise).
|
Pushed 3 commits that make this branch actually work on all-hot (Strix) boxes and land the decode optimizations, all validated on lucebox2 (DS4-Flash ROCMFP2, gfx1151, ROCm 7.2.2). Two bugs in the branch as previously pushed:
Byte-identical optimizations (default ON): persistent HC matvec pool, f16c dot kernels with scalar-order adds, shared per-step decode inputs (~430 tensor uploads/step → 2). Per-row accumulation order is unchanged everywhere, and output was verified byte-identical to the optimization-free path across repeated 200-token runs. Steady-state decode: 14.2 → 17.6-18.5 tok/s. Opt-in flags (default OFF, deterministic but NOT bit-identical — they reorder fp accumulation and change sampled text at temp 0):
Benchmarking note: the Strix iGPU sags from 2900 to 1473 MHz under sustained decode, which makes single-request numbers a lottery (14-18 tok/s from the same binary). Pin |
|
Measured |
The fused graph froze the attention window of layers without a mask segment (ratio-128 layers before token 128): they took the non-masked KV-assembly branch, whose shape depends on the live raw-row count, while the slot key assumed shape stability. On slot reuse the model never saw tokens generated after the graph was built, degenerating into repetition. Every layer now gets a mask-bundle segment (n_swa + padded comp rows, padded may be 0) and takes the masked full-ring attention branch. Also: - DFLASH_DS4_FUSED_DEBUG=1: dual-path mode that runs fused + per-layer reference on the same token and prints per-step argmax and max logit diff (this is what localized the bug) - DFLASH_DS4_FUSED_STABLE_GRAPH=1 (default OFF): single topology across flush/non-flush steps via an input-redirected state flush; prerequisite for CUDA/HIP graph replay, which measured as a net loss on gfx1151 Fused decode now: 18.6-19.0 tok/s coherent, per-step argmax matches the reference path; 18.9-19.15 with DFLASH_DS4_FFN_RAW_MMID=1 and DFLASH_DS4_FFN_FUSED_COMBINE=1 stacked.
|
Fused decode fixed and validated (069498e). Root cause of the quality regression: layers without a mask segment took the non-masked attention branch whose shape depends on the live raw-row count, so on graph-slot reuse their attention window stayed frozen at build size and the model never saw its own generated tokens. Every layer now uses the masked full-ring branch, making the fused graph truly position-independent. Numbers on lucebox2 (steady state, pinned clocks):
Also investigated HIP graph replay ( |
Compressor state double-buffers and compressed-KV rows retained the previous request's values (counters reset, contents did not), so the first flush windows of each request pooled over leftover state and outputs could drift by a token or two from the 2nd request on. Clearing the cache buffer at kv_offset==0 makes every subsequent request byte-stable (requests 2..N now produce identical output).
The generic sinkhorn kernel spilled its c[] matrix to scratch memory, costing 95us x 86 launches per decoded token (~8ms/token). The unrolled template<NHC=4> variant keeps it in registers (5.3us measured). DS4-Flash fused decode on Strix gfx1151: 19.1 -> 23.0 tok/s.
|
Two more wins pushed (35d0b05, 3ae984a + ggml 27597a9d7): Fused decode is now 23.0 tok/s (was 19.1). rocprofv3 showed Cross-request drift fixed (35d0b05): compressor state buffers kept the previous request's contents (only the counters were reset), so each request's first flush windows pooled over leftovers and outputs could flip a token from request 2 on. The cache buffer is now cleared at new-sequence prefill; requests 2..N are byte-identical to each other in every configuration. Remaining nuance: request 1 on a fresh process can differ from the converged steady state in one region (~token 15), both continuations coherent; under investigation. Current numbers (steady state, pinned clocks):
Baseline at the start of this work was 14.2 tok/s. |
…ctness) Chunked prefill is only exact while the whole prompt fits inside the raw SWA ring: the chunk graph writes ring slots pos % n_swa while attention reads the ring in the same graph, and the learned compressor only runs for the chunk's last token. Prompts beyond ~128 tokens degraded into incoherence (verified: a 344-token prompt produced degenerate output on every path; multi-turn conversations broke at turn 3), and --chunk 128 does not help. Token-by-token prefill matches the reference semantics at any length: the same 344-token prompt now yields the identical correct answer as its short version, and multi-turn conversations stay coherent. Chunked prefill remains available via DFLASH_DS4_CHUNKED_PREFILL=1 for short-prompt benchmarking. Cost: TTFT = prompt_tokens / ~20 tok/s until a banded-window prefill lands. Decode speed unaffected (22.9-23.0 steady with fused + FFN flags).
|
Superseded by #502 (vendored ROCmFPX ggml) + #503 (DS4 server changes). This PR bumped the
Verified on lucebox2 (gfx1151): the stack builds clean on HIP and holds decode at 22.8-22.9 tok/s (baseline 22.8-22.9). |
Server-side DeepSeek V4 Flash changes from #494, rebased onto the vendored ROCmFPX ggml tree (no submodule pointer). Byte-identical to the validated #494 server files. Correctness (default ON): - Token-by-token prefill by default; chunked prefill only fits inside the raw SWA ring, so long prompts / multi-turn degraded. Chunked stays available via DFLASH_DS4_CHUNKED_PREFILL for short-prompt benchmarking. - Clear the cache buffer at new-sequence prefill (kv_offset==0) so requests 2..N are byte-stable instead of pooling over leftover compressor state. - Route non-hybrid (all-hot) placement through the HC-complete layer-range path; deepseek4_step's non-hybrid branch is HC-less and generates garbage. - Key the cached decode-attn graph on the flush pattern; the old key collided once the compressed-KV ring filled, reusing a stale-topology graph. - Compressor decode graphs read state/comp-cache through the ggml_set_rows result tensors so the current-step writes are explicit graph dependencies. - Default DeepSeek4 chat prefix when the request has no system message, so the ROCmFPX "Src" GGUF stops behaving like a base model under bare prompts (explicit caller system prompts are preserved). Perf, byte-identical default: - Persistent HC matvec pool (row-split preserves per-row accumulation order) and f16c dot kernels with scalar-order adds. - Per-step decode scalar inputs uploaded in 2 tensor_sets instead of ~430. Opt-in, default OFF (documented as not bit-identical): - DFLASH_DS4_FUSED_DECODE single-graph decode with GGML_OP_DS4_HC. - DFLASH_DS4_FFN_RAW_MMID / DFLASH_DS4_FFN_FUSED_COMBINE accumulation reorder. - DFLASH_DS4_ROCMFPX_HC_GPU GPU HC pre-mix.
Server-side DeepSeek V4 Flash changes from #494, rebased onto the vendored ROCmFPX ggml tree (no submodule pointer). Byte-identical to the validated #494 server files. Correctness (default ON): - Token-by-token prefill by default; chunked prefill only fits inside the raw SWA ring, so long prompts / multi-turn degraded. Chunked stays available via DFLASH_DS4_CHUNKED_PREFILL for short-prompt benchmarking. - Clear the cache buffer at new-sequence prefill (kv_offset==0) so requests 2..N are byte-stable instead of pooling over leftover compressor state. - Route non-hybrid (all-hot) placement through the HC-complete layer-range path; deepseek4_step's non-hybrid branch is HC-less and generates garbage. - Key the cached decode-attn graph on the flush pattern; the old key collided once the compressed-KV ring filled, reusing a stale-topology graph. - Compressor decode graphs read state/comp-cache through the ggml_set_rows result tensors so the current-step writes are explicit graph dependencies. - Default DeepSeek4 chat prefix when the request has no system message, so the ROCmFPX "Src" GGUF stops behaving like a base model under bare prompts (explicit caller system prompts are preserved). Perf, byte-identical default: - Persistent HC matvec pool (row-split preserves per-row accumulation order) and f16c dot kernels with scalar-order adds. - Per-step decode scalar inputs uploaded in 2 tensor_sets instead of ~430. Opt-in, default OFF (documented as not bit-identical): - DFLASH_DS4_FUSED_DECODE single-graph decode with GGML_OP_DS4_HC. - DFLASH_DS4_FFN_RAW_MMID / DFLASH_DS4_FFN_FUSED_COMBINE accumulation reorder. - DFLASH_DS4_ROCMFPX_HC_GPU GPU HC pre-mix.
1 participant
Summary
This updates PR #494 after Howard's DeepSeek4 backend landed on
main.The previous PR history included ROCmFPX/HC fast-path experiments that were not byte-identical and could produce divergent or bad generation. This branch has been rebuilt from current
mainand now contains only the byte-safe fixes we want to carry forward.Changes
server/deps/llama.cppat the currentmainsubmodule pointer.ggml_set_rowsresult tensors, so current-step compressor writes are explicit graph dependencies before attention consumes compressed KV.DeepSeek V4 Flash SrcGGUF behaving like a source/base model under bare chat prompts, while preserving explicit caller system prompts.What Is Intentionally Not Included
ggml_ds4_hc_pre_directAPI.Validation
Validated on
lucebox2/ Strix Halo with:/home/lucebox2/models/DeepSeek-V4-Flash-ROCMFP2-STRIX.ggufRelevant final log:
/tmp/ds4bench_codex_rocm_quality_fixed.logSmoke prompts after the generation fix:
Answer only with the number: 123 + 268 =391,finish=stopAnswer one word: what color is the sky on a clear day?blue,finish=stopWrite exactly: hellohello,finish=stopWhat is 123 + 268?391,finish=stopThe EOS stop behavior itself is already present on
mainvia Howard's merged DS4 code (deepseek4_is_eos_tok). This PR only adds the missing chat prefix and graph dependency cleanup on top.Local checks:
git diff --check origin/main...HEADserver/src/deepseek4/deepseek4_graph.cppandserver/src/server/chat_template.cpp