feat(ds4): add monolithic Strix Halo concurrent serving - #598
Conversation
f5e8a26 to
17600d8
Compare
|
@cubic-dev-ai review this PR |
@Graffioh I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 25 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/src/deepseek4/deepseek4_graph.cpp">
<violation number="1" location="server/src/deepseek4/deepseek4_graph.cpp:7249">
P2: For long-running ratio-4 lanes, crossing each compression boundary rebuilds the whole-model graph instead of replaying it, causing recurring latency spikes and reducing serving throughput. Use a capacity-padded compressed-history topology with runtime visibility masking, or otherwise keep the graph shape stable across row-count changes.</violation>
</file>
<file name="server/src/common/concurrency/seq_slot_manager.cpp">
<violation number="1" location="server/src/common/concurrency/seq_slot_manager.cpp:175">
P2: When a stochastic request uses `seed=0`, this branch replaces the requested deterministic stream with `std::random_device`, so repeated requests cannot reproduce. Use `needs_logit_processing()` alone as the seed gate and seed the RNG with zero.</violation>
</file>
<file name="server/src/deepseek4/deepseek4_backend.cpp">
<violation number="1" location="server/src/deepseek4/deepseek4_backend.cpp:527">
P2: When hybrid placement uses ratio-4 layers, `estimate_ds4_cache_bytes` underestimates the indexer compressor state by one factor of two. Charge the actual `2 * index_dim` state width so the expert budget does not leave the subsequent cache allocation without enough VRAM.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| cache.plan.physical_blocks, ratio, prepared[(size_t) il])) return false; | ||
| for (const auto & row : prepared[(size_t) il]) { | ||
| key.push_back((int64_t) row.raw_history.size()); | ||
| key.push_back((int64_t) row.compressed_history.size()); |
There was a problem hiding this comment.
P2: For long-running ratio-4 lanes, crossing each compression boundary rebuilds the whole-model graph instead of replaying it, causing recurring latency spikes and reducing serving throughput. Use a capacity-padded compressed-history topology with runtime visibility masking, or otherwise keep the graph shape stable across row-count changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_graph.cpp, line 7249:
<comment>For long-running ratio-4 lanes, crossing each compression boundary rebuilds the whole-model graph instead of replaying it, causing recurring latency spikes and reducing serving throughput. Use a capacity-padded compressed-history topology with runtime visibility masking, or otherwise keep the graph shape stable across row-count changes.</comment>
<file context>
@@ -6854,6 +7156,259 @@ static bool initialize_layer_range_cache(
+ cache.plan.physical_blocks, ratio, prepared[(size_t) il])) return false;
+ for (const auto & row : prepared[(size_t) il]) {
+ key.push_back((int64_t) row.raw_history.size());
+ key.push_back((int64_t) row.compressed_history.size());
+ key.push_back(row.slot < 0 ? -1 :
+ (ratio ? row.position % ratio : row.position % DS4_PAGE_TOKENS));
</file context>
| s.sample_history = prompt; | ||
| // Same predicate the engine uses to pick CPU sampling over GPU argmax: | ||
| // a seed only means anything when the sampler actually draws. | ||
| if (sampler.needs_logit_processing() && sampler.seed != 0) { |
There was a problem hiding this comment.
P2: When a stochastic request uses seed=0, this branch replaces the requested deterministic stream with std::random_device, so repeated requests cannot reproduce. Use needs_logit_processing() alone as the seed gate and seed the RNG with zero.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/common/concurrency/seq_slot_manager.cpp, line 175:
<comment>When a stochastic request uses `seed=0`, this branch replaces the requested deterministic stream with `std::random_device`, so repeated requests cannot reproduce. Use `needs_logit_processing()` alone as the seed gate and seed the RNG with zero.</comment>
<file context>
@@ -0,0 +1,278 @@
+ s.sample_history = prompt;
+ // Same predicate the engine uses to pick CPU sampling over GPU argmax:
+ // a seed only means anything when the sampler actually draws.
+ if (sampler.needs_logit_processing() && sampler.seed != 0) {
+ s.rng.seed(sampler.seed);
+ } else {
</file context>
| // only in fixed-size state scratch and does not scale with context. | ||
| const size_t index_dim = (size_t) w.n_indexer_head_dim; | ||
| total_bytes += comp_cap * index_dim * sizeof(uint16_t); | ||
| total_bytes += state_rows * index_dim * sizeof(float) * 2; |
There was a problem hiding this comment.
P2: When hybrid placement uses ratio-4 layers, estimate_ds4_cache_bytes underestimates the indexer compressor state by one factor of two. Charge the actual 2 * index_dim state width so the expert budget does not leave the subsequent cache allocation without enough VRAM.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_backend.cpp, line 527:
<comment>When hybrid placement uses ratio-4 layers, `estimate_ds4_cache_bytes` underestimates the indexer compressor state by one factor of two. Charge the actual `2 * index_dim` state width so the expert budget does not leave the subsequent cache allocation without enough VRAM.</comment>
<file context>
@@ -503,20 +515,19 @@ static uint64_t estimate_ds4_cache_bytes(const DeepSeek4Weights & w, int max_ctx
+ // only in fixed-size state scratch and does not scale with context.
+ const size_t index_dim = (size_t) w.n_indexer_head_dim;
+ total_bytes += comp_cap * index_dim * sizeof(uint16_t);
+ total_bytes += state_rows * index_dim * sizeof(float) * 2;
+ total_bytes += (size_t) 2 * 2 * ratio *
+ (size_t) w.n_indexer_head * index_dim *
</file context>
| total_bytes += state_rows * index_dim * sizeof(float) * 2; | |
| total_bytes += state_rows * (2 * index_dim) * sizeof(float) * 2; |
Scope
This rebuilds the concurrency work for DeepSeek4 only. Qwen source and GGML kernels are unchanged.
It supersedes the experimental direction in #594, #595, #596 and Graffioh#16, while incorporating the MoE concurrency discussion. The supported topology is deliberately narrow: one local HIP
gfx1151Strix Halo with the complete model and all experts resident.Implementation
SeqEngineboundary with admission, cancellation, fair prompt progress, slow-client isolation, and reusable slot lifecycle handling.--max-concurrency 2..16and exposes optional physical-pool sizing through--kv-pool-tokens.Guardrails
This path fails closed for CUDA, non-
gfx1151HIP devices, layer or remote target splits,DFLASH_DS4_MOE_TP, drafts/DSpark, DDTree, PFlash/KVFlash, fused decode, approximate prefill, windowed attention, and prefix-cache parking. It requires the full model to remain resident on the single Strix Halo.Strix Halo concurrency screening
Model GGUF:
DeepSeek-V4-Flash-ROCMFP2-STRIX.ggufDevice: one local HIP
gfx1151Strix Halo (Radeon 8060S), with the full model and experts resident.Protocol: one fresh server and same-concurrency warm-up per level; short ragged, disjoint prompts; exact prefill; greedy streaming; 256 forced completion tokens per request; one repeat.
No llama.cpp comparison is stated yet: the available
llama-serverrejects this ROCmFP GGUF at load time (invalid ggml type 101). A compatible llama.cpp/DeepSeek4 ROCmFP build (or a compatible GGUF) is required before publishing paired DS4-vs-llama results. This is a one-repeat screening, not a performance claim; use five paired repeats once that baseline is available.Validation
gfx1151server and all focused concurrency targets, includingtest_deepseek4_unit.