fix(ds4): correct layer-split HC handoff and per-device state#507
Conversation
|
We plan to run two AMD GPUs in the same HIP process. g_scratch is currently shared across devices, so the second shard may reuse scratch memory allocated on the first GPU. Please keep one scratch buffer per HIP device, set the correct device in HC worker threads |
bf8dc45 to
ecf55b1
Compare
581817d to
b6a14e6
Compare
0441ac2 to
4ae83dd
Compare
e943f2a to
d5fae56
Compare
4ae83dd to
796299e
Compare
7d6c85c to
5a3b84c
Compare
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mx99uuhcriPweAFggPwfce
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mx99uuhcriPweAFggPwfce
5a3b84c to
2c38433
Compare
Also size the shared scratch for the requested shapes: the bounded buffers (mix/post/comb) are allocated at their kMaxHc maxima and the working buffer tracks n_embd capacity, instead of staying at whatever dims the first call happened to use. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mx99uuhcriPweAFggPwfce
2c38433 to
685de03
Compare
| std::fprintf(stderr, g_failures ? " done\n" : " ok\n"); | ||
| } | ||
|
|
||
| static void test_hc_set_device_contract() { |
There was a problem hiding this comment.
(non-blocking) idk if needed or can be removed
There was a problem hiding this comment.
1 issue found across 11 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
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_hc_cuda.cu">
<violation number="1" location="server/src/deepseek4/deepseek4_hc_cuda.cu:97">
P3: Concurrent HC calls remain unverified: the per-device test runs device workers sequentially, so it cannot validate simultaneous slot creation and use. Add a barrier-based two-device invocation before treating per-device scratch isolation as covered.</violation>
</file>
| // were created. Keep one independently locked slot per logical device so local | ||
| // shards cannot reuse another device's scratch pointers. | ||
| std::mutex g_scratch_slots_mutex; | ||
| std::vector<std::unique_ptr<HcScratchSlot>> g_scratch_slots; |
There was a problem hiding this comment.
P3: Concurrent HC calls remain unverified: the per-device test runs device workers sequentially, so it cannot validate simultaneous slot creation and use. Add a barrier-based two-device invocation before treating per-device scratch isolation as covered.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_hc_cuda.cu, line 97:
<comment>Concurrent HC calls remain unverified: the per-device test runs device workers sequentially, so it cannot validate simultaneous slot creation and use. Add a barrier-based two-device invocation before treating per-device scratch isolation as covered.</comment>
<file context>
@@ -62,8 +81,38 @@ struct HcCudaScratch {
+// were created. Keep one independently locked slot per logical device so local
+// shards cannot reuse another device's scratch pointers.
+std::mutex g_scratch_slots_mutex;
+std::vector<std::unique_ptr<HcScratchSlot>> g_scratch_slots;
+
+HcScratchSlot * current_scratch_slot() {
</file context>
There was a problem hiding this comment.
non-blocking for this PR to be merged
how do we handle concurrency tests? in integration tests? @davide221
There was a problem hiding this comment.
We don’t currently support concurrent generation requests (support coming in August). So non blocking for now
Reset the backing DeepSeek4 cache buffers for both local and IPC shards instead of only rewinding their counters, and clear stale prefill logits on a fresh request. Extend the adapter reset regression to verify cache tensors are zeroed. Also size the HC reference Sinkhorn scratch from n_hc so the new n_hc=8 capacity test does not overflow its stack buffer.
Summary
Fixes correctness and device-isolation issues in the DeepSeek4 HC layer-split path:
Implementation
Local layer split now follows the full HC flow:
Runtime caches are owned by each shard and keyed by weights, context, backend, device, layer range, and output ownership. HC scratch is stored in independently locked slots indexed by CUDA/HIP device.
Follow-up
The opt-in GPU F16 HC fn mirror for quantized ROCmFPX fn weights (
DFLASH_DS4_ROCMFPX_HC_GPU) was split out into #546, stacked on this PR, including thedeepseek4_cuda_hc_upload_f16/deepseek4_cuda_hc_freeprimitives.Validation
git diff --checkand C++ syntax checks passedtest_deepseek4_unitanddflash_server; single-shard and virtual two-shard[0,22)/[22,43)runs oncuda:0completed sequential requests without HC, CUDA, crash, or abort errors, validating HC handoff and per-shard cache reuse — re-run on the current head pendingdevice 0 -> device 1 -> device 0validation on lucebox3/lucebox5 is still required