[CI][PD] Add optional/nightly DSv4 Disaggregated eval

[NickLucche]

As per-title, add DSv4 8 gpus tests. Optional as we don't want to run this gpu-hungry setup on each PR.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[gemini-code-assist]

Code Review

This pull request introduces a new disaggregated DP EP test case for the DeepSeek-V4-Flash model in the Buildkite configuration and updates the model's accuracy threshold in the integration tests. Feedback was provided to include a source_file_dependencies block in the new test step to ensure consistency with other steps and proper CI cache management.

[gemini-code-assist]

The new test step is missing the source_file_dependencies block. This is inconsistent with all other steps in this file and is important for the CI system to correctly track changes and manage cache invalidation for this test.

- label: DSv4-Flash Disaggregated DP EP
  key: dsv4-flash-disaggregated
  timeout_in_minutes: 30
  device: h200
  optional: true
  working_dir: "/vllm-workspace/tests"
  num_devices: 8
  source_file_dependencies:
    - vllm/distributed/kv_transfer/kv_connector/v1/nixl/
    - tests/v1/kv_connector/nixl_integration/
  env:
    ENABLE_HMA_FLAG: "1"
    DP_EP: "1"
    GPU_MEMORY_UTILIZATION: "0.85"
    PREFILLER_TP_SIZE: "4"
    DECODER_TP_SIZE: "4"
    PREFILL_BLOCK_SIZE: "256"
    DECODE_BLOCK_SIZE: "256"
    MODEL_NAMES: "deepseek-ai/DeepSeek-V4-Flash"
    VLLM_SERVE_EXTRA_ARGS: "--trust-remote-code,--kv-cache-dtype,fp8"
  commands:
    - uv pip install --system -r /vllm-workspace/requirements/kv_connectors.txt
    - bash v1/kv_connector/nixl_integration/run_accuracy_test.sh

[NickLucche]

@claude review

[claude]

🔴 The new TP=4/4 configuration is the first call site of run_accuracy_test.sh to exercise its inner GPU-ID build loop more than once, exposing a latent Bash bug at tests/v1/kv_connector/nixl_integration/run_accuracy_test.sh:142-204. Once GPU_ID becomes a comma-separated string, $(((GPU_ID + j) % NUM_GPUS)) triggers Bash's C-style comma operator, so prefill ends up with CUDA_VISIBLE_DEVICES=0,1,3,6 and decode with 7,0,2,5 — both collide on GPU 0 at gpu-memory-utilization=0.85 and the optional DSv4 step will OOM rather than produce accuracy numbers. Fix in run_accuracy_test.sh: accumulate GPU IDs in a Bash array and stringify only at the end, and reset NEXT_GPU between the prefill and decode loops (this is pre-existing, but only this PR triggers it).

Extended reasoning...

Bug

The optional DSv4-Flash CI step added here is the first call site of tests/v1/kv_connector/nixl_integration/run_accuracy_test.sh to use PREFILLER_TP_SIZE / DECODER_TP_SIZE > 2. That configuration exposes a latent Bash bug in the script that builds CUDA_VISIBLE_DEVICES for each prefill/decode instance.

The relevant code (lines 142-150 for prefill, mirrored at 197-204 for decode) is:

GPU_ID=$((i % $(get_num_gpus)))
NEXT_GPU=${GPU_ID}
for (( j=1; j < PREFILLER_TP_SIZE; j++ )); do
  NEXT_GPU=$(((GPU_ID + j) % $(get_num_gpus)))
  GPU_ID="${GPU_ID},${NEXT_GPU}"
done

After the first iteration GPU_ID is a comma-separated string (e.g. 0,1). On the next iteration the arithmetic expansion $(((GPU_ID + j) % N)) evaluates GPU_ID + j inside Bash arithmetic, where comma is the lowest-precedence C-style comma operator. The expression evaluates left-to-right and yields only the last sub-expression, so the prior tokens are discarded and +j is applied only to the trailing element.

Step-by-step proof

With NUM_GPUS=8, PREFILLER_TP_SIZE=4, i=0 (one prefill instance):

j	NEXT_GPU calculation	NEXT_GPU	GPU_ID
init	i%8	0	0
1	(0 + 1) % 8	1	0,1
2	(0,1 + 2) % 8 → comma op → (0, 1+2) → 3	3	0,1,3
3	(0,1,3 + 3) % 8 → (0, 1, 3+3) → 6	6	0,1,3,6

Prefill is launched with CUDA_VISIBLE_DEVICES=0,1,3,6 (intended: 0,1,2,3). After the prefill loop, NEXT_GPU=6 leaks into the decode loop because it is never reset. With DECODER_TP_SIZE=4 and i=0:

j	NEXT_GPU calculation	NEXT_GPU	GPU_ID
init	(0 + 6 + 1) % 8		7
1	(7 + 1) % 8	0	7,0
2	(7,0 + 2) % 8 → (7, 0+2) → 2	2	7,0,2
3	(7,0,2 + 3) % 8 → (7, 0, 2+3) → 5	5	7,0,2,5

Decode is launched with CUDA_VISIBLE_DEVICES=7,0,2,5 (intended: 4,5,6,7). The disjoint allocations {0,1,2,3}/{4,5,6,7} implied by the comment "starting from after prefill GPUs" at line 198 are not what the loop actually produces.

Impact

Both prefill and decode processes are launched in parallel via eval "$FULL_CMD &" at lines 189 and 251. Both sets contain GPU 0, and both serve commands set --gpu-memory-utilization $GPU_MEMORY_UTILIZATION with GPU_MEMORY_UTILIZATION=0.85 from the new YAML — they will contend on / OOM GPU 0 (and waste GPU 4 entirely). The new optional CI step therefore cannot produce valid DSv4 accuracy numbers as configured, defeating its purpose.

Why this is the first trigger

Every other caller of run_accuracy_test.sh — config_sweep_accuracy_test.sh (tp_configs, dp_ep_configs, hybrid_ssm_configs) and all other YAML steps in this file — uses PREFILLER_TP_SIZE / DECODER_TP_SIZE ≤ 2. With TP=2 the j loop runs exactly once, GPU_ID is still a plain integer when the arithmetic happens, and the comma-operator path is never exercised. The TP=4/4 configuration introduced by this PR is the first to expose the latent bug, so flagging it here is appropriate even though the broken code lives in another file.

Fix

Two changes in run_accuracy_test.sh:

1. Accumulate GPU IDs in a Bash array and stringify only at the end, e.g.:

   GPUS=("$((i % $(get_num_gpus)))")
   for (( j=1; j < PREFILLER_TP_SIZE; j++ )); do
     GPUS+=("$(((${GPUS[0]} + j) % $(get_num_gpus)))")
   done
   GPU_ID=$(IFS=,; echo "${GPUS[*]}")
   NEXT_GPU=${GPUS[-1]}

2. Reset / recompute NEXT_GPU at the start of the decode loop instead of leaking it from prefill.

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @NickLucche.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @NickLucche.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[NickLucche]

New nightly test is passing on CI

========================================================== 1 passed, 16 warnings in 409.29s (0:06:49) ==========================================================
--
  | sys:1: DeprecationWarning: builtin type swigvarlink has no __module__ attribute
  | + cleanup_instances
  | + echo 'Cleaning up any running vLLM instances...'
  | Cleaning up any running vLLM instances...
  | + pkill -f 'vllm serve'
  | + sleep 2
  |  

[AndreasKaratzas]

LGTM

We might mirror this too for AMD in a follow-up PR. It's an important model.