Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6efd246
fix(changelog): accept single-node agentic eval rows in matrix valida…
majunze2001 Jul 17, 2026
a53a365
[AgentX] DeepSeek-V4 B200 vLLM: add MTP (num_speculative_tokens=3) fo…
majunze2001 Jul 17, 2026
83ab93a
docs(changelog): add B200 vLLM MTP entry (PR #2259)
majunze2001 Jul 17, 2026
026711b
[AgentX] B200 vLLM: split MTP into a separate config-key
majunze2001 Jul 17, 2026
bebd5e6
align synthetic acceptance length to 2.49
majunze2001 Jul 17, 2026
894d1b5
Merge remote-tracking branch 'origin/main' into agentx/dsv4-b200-vllm…
majunze2001 Jul 17, 2026
5187947
reduce GPU memory utilization from 0.92 to 0.9
majunze2001 Jul 17, 2026
a32d07f
Revert "fix(changelog): accept single-node agentic eval rows in matri…
majunze2001 Jul 18, 2026
8617971
Merge remote-tracking branch 'origin/main' into agentx/dsv4-b200-vllm…
majunze2001 Jul 18, 2026
425fd86
MTP synthetic acceptance for throughput, real verification for eval
majunze2001 Jul 18, 2026
0511701
Merge remote-tracking branch 'origin' into agentx/dsv4-b200-vllm-mtp
majunze2001 Jul 18, 2026
4b5cb05
add missing tool call parser for swe eval
majunze2001 Jul 19, 2026
32ce011
Merge branch 'main' into agentx/dsv4-b200-vllm-mtp
majunze2001 Jul 19, 2026
fc1c584
fix(agentic): preserve B200 MTP PCP GPU count
cquil11 Jul 20, 2026
579c16f
fix(agentic): derive B200 MTP GPU count from topology
cquil11 Jul 20, 2026
bc4065e
Merge remote-tracking branch 'origin/main' into agentx/dsv4-b200-vllm…
cquil11 Jul 20, 2026
607e14e
Merge remote-tracking branch 'origin/main' into agentx/dsv4-b200-vllm…
cquil11 Jul 22, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
302 changes: 302 additions & 0 deletions benchmarks/single_node/agentic/dsv4_fp4_b200_vllm_mtp.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,302 @@
#!/usr/bin/env bash
set -eo pipefail
set -x

# Agentic trace replay benchmark for DeepSeek-V4-Pro FP4 on B200 using vLLM,
# with MTP speculative decoding (num_speculative_tokens=3).
#
# Identical to dsv4_fp4_b200_vllm.sh (same image, engine args, offload, GPU
# topologies, and agentic aiperf rig) with exactly two MTP deltas:
# --speculative-config '{"method": "mtp", "num_speculative_tokens": 3}'
# --max-cudagraph-capture-size expressed in TOKENS (see the capture block below).
#
# Mirrors the fixed-seq-len parallelism options (pure TP and DEP) so the
# agentic sweep can probe both interactivity and throughput regimes:
# pure TP (DP_ATTENTION=false, EP_SIZE=1): attention TP-sharded across
# all $TP GPUs in a single engine. Lower TPOT, lower batch.
# TP+EP (DP_ATTENTION=false, EP_SIZE>1): attention TP-sharded, MoE
# experts EP-sharded within the TP group.
# DEP (DP_ATTENTION=true, EP_SIZE>1): per-DP-rank attention with
# experts EP-sharded across DP ranks (per the vLLM blog recipe).
# Highest aggregate throughput at large CONC.
#
# Image is configured in nvidia-master.yaml. block_size=256,
# kv-cache-dtype=fp8, FLASHINFER_MLA_SPARSE_DSV4 attention with the FP4 indexer
# cache, FULL_DECODE_ONLY cudagraph capture, and (in EP tiers) mega-MoE backend.
#
# Required env vars:
# MODEL, TP, CONC, KV_OFFLOADING, TOTAL_CPU_DRAM_GB, RESULT_DIR
#
# Pure TP is GPU-resident (KV_OFFLOADING=none). DEP tiers offload KV to host
# DRAM: KV_OFFLOADING=dram requires KV_OFFLOAD_BACKEND=vllm-simple or mooncake.

source "$(dirname "$0")/../../benchmark_lib.sh"

check_env_vars MODEL TP CONC KV_OFFLOADING TOTAL_CPU_DRAM_GB RESULT_DIR DURATION EP_SIZE DP_ATTENTION

if [ -z "$DCP_SIZE" ]; then
DCP_SIZE=1
fi
if [ -z "$PCP_SIZE" ]; then
PCP_SIZE=1
fi
VLLM_CP_ARGS=()
if [ "$DCP_SIZE" -gt 1 ]; then
VLLM_CP_ARGS+=(--decode-context-parallel-size "$DCP_SIZE")
fi
if [ "$PCP_SIZE" -gt 1 ]; then
VLLM_CP_ARGS+=(--prefill-context-parallel-size "$PCP_SIZE")
fi

GPU_COUNT=$TP
if [[ ! "$GPU_COUNT" =~ ^[1-9][0-9]*$ ]]; then
echo "Error: GPU_COUNT must be a positive integer, got '$GPU_COUNT'" >&2
exit 1
fi
export GPU_COUNT

if [[ -n "$SLURM_JOB_ID" ]]; then
echo "JOB $SLURM_JOB_ID running on $SLURMD_NODENAME"
fi

# `hf download` creates the target dir if missing and is itself idempotent.
# When MODEL_PATH is unset (stand-alone runs), fall back to the HF_HUB_CACHE
# Either way, MODEL_PATH is what the server is launched with.
if [[ -n "$MODEL_PATH" ]]; then
if [[ ! -d "$MODEL_PATH" || -z "$(ls -A "$MODEL_PATH" 2>/dev/null)" ]]; then
hf download "$MODEL" --local-dir "$MODEL_PATH"
fi
else
hf download "$MODEL"
export MODEL_PATH="$MODEL"
fi
nvidia-smi

# ---- Resolve traces and install deps ----------------------------------------
resolve_trace_source
install_agentic_deps

# vllm-project/router expands the one HTTP backend into one logical worker per
# DP rank and sends X-data-parallel-rank on forwarded requests. aiperf's
# X-Correlation-ID is stable for every turn of a conversation; alias it to the
# router's preferred X-Session-ID header.
USE_VLLM_ROUTER=false
VLLM_BACKEND_PORT="$PORT"
if [ "$DP_ATTENTION" = "true" ]; then
USE_VLLM_ROUTER=true
VLLM_BACKEND_PORT=$((PORT + 1))
VLLM_ROUTER_VERSION=0.1.14
VLLM_ROUTER_POLICY=consistent_hash
VLLM_ROUTER_METRICS_PORT=$((PORT + 10000))
export AIPERF_HTTP_X_SESSION_ID_FROM_CORRELATION_ID=1
agentic_pip_install --quiet "vllm-router==$VLLM_ROUTER_VERSION"
fi

# DeepSeek-V4-Pro weights are large; engine startup can exceed default 600s.
export VLLM_ENGINE_READY_TIMEOUT_S=3600

# vllm-project/vllm#43447 keeps local SWA prefix-cache tails sparsely, while
# vllm-project/vllm#44774 applies the same reachability policy to Mooncake's
# store mask. 32k matches the trace-replay tuning validated for this workload.
export VLLM_PREFIX_CACHE_RETENTION_INTERVAL=32768
export VLLM_USE_V2_MODEL_RUNNER=1
export VLLM_USE_RUST_FRONTEND=1
export VLLM_DSV4_MEGA_FP8_COMBINE=1
export VLLM_RPC_TIMEOUT=600000

# ---- Server config ----------------------------------------------------------
SERVER_LOG="$RESULT_DIR/server.log"
ROUTER_LOG="$RESULT_DIR/router.log"
MOONCAKE_MASTER_LOG="$RESULT_DIR/mooncake_master.log"
mkdir -p "$RESULT_DIR"

SERVER_PID=""
ROUTER_PID=""
MOONCAKE_MASTER_PID=""

OFFLOAD_ARGS=()
case "$KV_OFFLOAD_BACKEND" in
"")
require_agentic_kv_offload_none
;;
vllm-simple)
require_agentic_kv_offload_backend vllm-simple
CPU_BYTES_PER_RANK=$(( TOTAL_CPU_DRAM_GB * 1000 * 1000 * 1000 / GPU_COUNT ))
# Identical prefixes must hash to identical block keys across DP ranks.
export PYTHONHASHSEED=42
OFFLOAD_CONFIG=$(cat <<EOF
{
"kv_connector": "SimpleCPUOffloadConnector",
"kv_role": "kv_both",
"kv_connector_extra_config": {
"cpu_bytes_to_use_per_rank": ${CPU_BYTES_PER_RANK},
"lazy_offload": false,
"enable_cross_layers_blocks": "true"
}
}
EOF
)
OFFLOAD_ARGS=(
--kv-transfer-config
"$OFFLOAD_CONFIG"
)
;;
mooncake)
require_agentic_kv_offload_backend mooncake
# Embedded mode contributes one segment per GPU rank to a shared
# distributed store, so pre-divide the aggregate host-memory budget.
PER_RANK_GB=$((TOTAL_CPU_DRAM_GB / GPU_COUNT))

MOONCAKE_VERSION=0.3.11.post1
agentic_pip_install --quiet --no-cache-dir --no-deps \
--force-reinstall "mooncake-transfer-engine-cuda13==$MOONCAKE_VERSION"
python3 -c "from mooncake.store import MooncakeDistributedStore" >/dev/null

MOONCAKE_MASTER_PORT=$((PORT + 12000))
MOONCAKE_CONFIG_PATH="$RESULT_DIR/mooncake_config.json"
cat > "$MOONCAKE_CONFIG_PATH" <<EOF
{
"mode": "embedded",
"metadata_server": "P2PHANDSHAKE",
"master_server_address": "127.0.0.1:$MOONCAKE_MASTER_PORT",
"global_segment_size": "${PER_RANK_GB}GB",
"local_buffer_size": "4GB",
"protocol": "rdma",
"device_name": "mlx5_0,mlx5_1,mlx5_2,mlx5_3,mlx5_4,mlx5_5,mlx5_10,mlx5_11",
"enable_offload": false
}
EOF
export MOONCAKE_CONFIG_PATH
export MC_ENABLE_DEST_DEVICE_AFFINITY=1
# Identical prefixes must hash to identical store keys across DP ranks.
export PYTHONHASHSEED=0
export WITH_NVIDIA_PEERMEM=0
export MC_SLICE_SIZE=1048576
export MC_WORKERS_PER_CTX=4

# Each rank contributes a separate segment. Evict early enough to
# avoid an imbalanced rank exhausting its segment.
MOONCAKE_EVICTION_HIGH_WATERMARK_RATIO=0.80
MOONCAKE_EVICTION_RATIO=0.10
# Mooncake's default 5s read lease is shorter than the observed
# transfer latency for large DSv4 hybrid-KV loads on B200 TCP.
MOONCAKE_KV_LEASE_TTL=60s

echo "Starting Mooncake master on port $MOONCAKE_MASTER_PORT..."
mooncake_master --port "$MOONCAKE_MASTER_PORT" \
--eviction_high_watermark_ratio="$MOONCAKE_EVICTION_HIGH_WATERMARK_RATIO" \
--eviction_ratio="$MOONCAKE_EVICTION_RATIO" \
--default_kv_lease_ttl="$MOONCAKE_KV_LEASE_TTL" \
> "$MOONCAKE_MASTER_LOG" 2>&1 &
MOONCAKE_MASTER_PID=$!
sleep 2
if ! kill -0 "$MOONCAKE_MASTER_PID" 2>/dev/null; then
echo "Mooncake master died during startup." >&2
cat "$MOONCAKE_MASTER_LOG" >&2
exit 1
fi
unset VLLM_USE_SIMPLE_KV_OFFLOAD
OFFLOAD_ARGS=(
--kv-transfer-config
'{"kv_connector":"MooncakeStoreConnector","kv_role":"kv_both","kv_connector_extra_config":{"load_async":true}}'
)
;;
*)
echo "Error: unsupported B200 KV_OFFLOAD_BACKEND='$KV_OFFLOAD_BACKEND'" >&2
exit 1
;;
esac

PARALLEL_ARGS=(--tensor-parallel-size "$TP" --data-parallel-size 1)
if [ "$DP_ATTENTION" = "true" ]; then
PARALLEL_ARGS=(--tensor-parallel-size 1 --data-parallel-size "$TP")
fi

EP_ARGS=()
FAST_MOE_ARGS=()
if [ "$EP_SIZE" -gt 1 ]; then
EP_ARGS=(--enable-expert-parallel)
FAST_MOE_ARGS=(
--moe-backend deep_gemm_amxf4_mega_moe
--enable-ep-weight-filter
--prefill-schedule-interval 16
)
fi

# AgentX concurrency counts live session trees, not individual requests.
# Subagent fan-out can push instantaneous request concurrency above CONC, so
# leave 2x headroom rather than clipping those bursts at the scheduler.
MAX_NUM_SEQS=$((2 * CONC))

# MTP: cudagraph capture sizes are in TOKENS. A uniform decode batch of S seqs
# verifies S*(1+num_speculative_tokens) tokens, so cap capture at
# MAX_NUM_SEQS*(1+N) tokens -- otherwise vLLM's FULL_DECODE_ONLY ladder tops out
# at MAX_NUM_SEQS/(1+N) seqs and the largest decode batches fall back to eager.
NUM_SPEC_TOKENS=3
TOKENS_PER_SEQ=$((1 + NUM_SPEC_TOKENS))
MAX_CUDAGRAPH_CAPTURE_SIZE=$((MAX_NUM_SEQS * TOKENS_PER_SEQ))

echo "Starting vllm server..."
export TORCH_CUDA_ARCH_LIST="10.0"
export PYTHONNOUSERSITE=1
export VLLM_FLOAT32_MATMUL_PRECISION=high

{ set +x; } 2>/dev/null
VLLM_CMD=(
vllm serve "$MODEL_PATH" --served-model-name "$MODEL"
--host 0.0.0.0
--port "$VLLM_BACKEND_PORT"
--trust-remote-code
--kv-cache-dtype fp8
--block-size 256
--max-model-len 1048576
--gpu-memory-utilization 0.92
--numa-bind
--enable-cumem-allocator
--no-enable-flashinfer-autotune
--tokenizer-mode deepseek_v4
--reasoning-parser deepseek_v4
--attention-config '{"backend":"FLASHINFER_MLA_SPARSE_DSV4","use_prefill_query_quantization":true,"use_fp4_indexer_cache":true}'
--speculative-config "{\"method\": \"mtp\", \"num_speculative_tokens\": $NUM_SPEC_TOKENS}"
--no-disable-hybrid-kv-cache-manager
--disable-uvicorn-access-log
--compilation-config '{"cudagraph_mode":"FULL_DECODE_ONLY","mode":0}'
--max-num-seqs "$MAX_NUM_SEQS"

Check failure on line 264 in benchmarks/single_node/agentic/dsv4_fp4_b200_vllm_mtp.sh

View check run for this annotation

Claude / Claude Code Review

Agentic dsv4 MTP script uses real decoding instead of required synthetic acceptance pinned to golden AL curve

The new agentic script benchmarks/single_node/agentic/dsv4_fp4_b200_vllm_mtp.sh runs real (non-synthetic) MTP decoding instead of the fairness-mandated synthetic acceptance pinned to the golden AL curve. Its --speculative-config only sets method/num_speculative_tokens, missing rejection_sample_method=synthetic and synthetic_acceptance_length (2.49 thinking_on / 2.97 thinking_off from golden_al_distribution/dsv4_mtp.yaml), unlike the precedent kimik2.5_fp4_b300_mtp.sh script.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The new agentic script benchmarks/single_node/agentic/dsv4_fp4_b200_vllm_mtp.sh runs real (non-synthetic) MTP decoding instead of the fairness-mandated synthetic acceptance pinned to the golden AL curve. Its --speculative-config only sets method/num_speculative_tokens, missing rejection_sample_method=synthetic and synthetic_acceptance_length (2.49 thinking_on / 2.97 thinking_off from golden_al_distribution/dsv4_mtp.yaml), unlike the precedent kimik2.5_fp4_b300_mtp.sh script.

Extended reasoning...

The bug: docs/PR_REVIEW_CHECKLIST.md contains a mandatory, agentic-scoped CODEOWNER sign-off item (line 23):

For agentic workloads: verified that speculative-decoding configs (EAGLE / MTP / draft models) run with simulated synthetic acceptance, with the acceptance-length value taken from the committed golden AL curve in golden_al_distribution/ for that model, thinking mode, and draft length. A submission may choose any supported draft length, but it may not substitute a different acceptance target.

golden_al_distribution/README.md's "Fairness Guidelines for AgentX" section reinforces this and shows the exact vLLM mechanism: inject the golden value via rejection_sample_method: "synthetic" + synthetic_acceptance_length: <value> in --speculative-config. The committed curve at golden_al_distribution/dsv4_mtp.yaml gives deepseek-v4-pro, num_speculative_tokens=3 → AL 2.49 (thinking_on) / 2.97 (thinking_off).

Where it breaks: In the new file benchmarks/single_node/agentic/dsv4_fp4_b200_vllm_mtp.sh (line ~260), the --speculative-config passed to vllm serve is:

--speculative-config "{\"method\": \"mtp\", \"num_speculative_tokens\": $NUM_SPEC_TOKENS}"

This has no rejection_sample_method or synthetic_acceptance_length field, so vLLM performs real (non-synthetic) MTP verification against the actual draft model's predictions rather than sampling acceptance at a fixed, pinned rate.

Why nothing else catches it: The only other agentic MTP/EAGLE script in the repo, benchmarks/single_node/agentic/kimik2.5_fp4_b300_mtp.sh, correctly follows the fairness contract — it sets rejection_sample_method:"synthetic" and synthetic_acceptance_length (2.88 / 3.24, selected by DCP_SIZE branch) directly in its --speculative-config string. The new dsv4 script has no equivalent branch or env-var override anywhere in the file that injects these fields; VLLM_CMD is built once, inline, with the literal string above, so there is no other code path that could compensate.

Impact: Without pinning acceptance to the golden curve, the measured acceptance length (and therefore decode throughput/speedup from MTP) is free to drift with whatever the real draft model happens to achieve on the agentic replay traffic. This defeats the stated purpose of the golden-AL mechanism — making agentic MTP speedup numbers comparable across submissions/vendors — and is exactly the failure mode the checklist item and Fairness Guidelines were written to prevent. It is also a mandatory, explicit CODEOWNER sign-off checklist item, not a style preference.

Step-by-step proof:

  1. A reviewer runs the new pure-TP+MTP tier added in configs/nvidia-master.yaml (spec-decoding: mtp, conc-list: [1,2,4,6,8]).
  2. dsv4_fp4_b200_vllm_mtp.sh launches vllm serve with --speculative-config '{"method": "mtp", "num_speculative_tokens": 3}'.
  3. vLLM's MTP draft head proposes tokens and the target model verifies them for real — acceptance depends on true model/draft agreement on the actual agentic trace content, which is not controlled.
  4. Compare to kimik2.5_fp4_b300_mtp.sh: its config additionally carries "rejection_sample_method":"synthetic","synthetic_acceptance_length":2.88, forcing every decode step to accept exactly the pinned AL regardless of true draft/target agreement.
  5. Because dsv4's config lacks those two fields, its published throughput/TPOT numbers reflect whatever real AL the traces produce that day/run, not the committed 2.49/2.97 golden value — so DeepSeek-V4 MTP results are not apples-to-apples comparable to other vendors' agentic MTP submissions the way the fairness guideline requires.

Fix: Add rejection_sample_method: "synthetic" and synthetic_acceptance_length: <2.49 or 2.97> (selected based on thinking mode, mirroring how kimik2.5_fp4_b300_mtp.sh branches on DCP_SIZE) to the --speculative-config JSON at line ~260.

Note: fixed_seq_len/dsv4_fp4_b200_vllm_mtp.sh also uses real decoding, but the checklist item is explicitly scoped to agentic workloads, so that script is out of scope for this finding.

--max-cudagraph-capture-size "$MAX_CUDAGRAPH_CAPTURE_SIZE"
"${PARALLEL_ARGS[@]}"
"${VLLM_CP_ARGS[@]}"
"${EP_ARGS[@]}"
"${FAST_MOE_ARGS[@]}"
"${OFFLOAD_ARGS[@]}"
)
printf '%q ' "${VLLM_CMD[@]}" | tee "$RESULT_DIR/vllm_command.txt"
printf '\n' | tee -a "$RESULT_DIR/vllm_command.txt"
"${VLLM_CMD[@]}" > "$SERVER_LOG" 2>&1 &
SERVER_PID=$!
echo "Server PID: $SERVER_PID"

wait_for_server_ready --port "$VLLM_BACKEND_PORT" --server-log "$SERVER_LOG" --server-pid "$SERVER_PID"

if [ "$USE_VLLM_ROUTER" = "true" ]; then
echo "Starting native vLLM router on port $PORT for $TP DP ranks..."
vllm-router \
--worker-urls "http://localhost:$VLLM_BACKEND_PORT" \
--policy "$VLLM_ROUTER_POLICY" \
--intra-node-data-parallel-size "$TP" \
--host 0.0.0.0 \
--port "$PORT" \
--prometheus-host 127.0.0.1 \
--prometheus-port "$VLLM_ROUTER_METRICS_PORT" \
--request-timeout-secs 14400 \
--disable-retries > "$ROUTER_LOG" 2>&1 &
ROUTER_PID=$!
echo "Router PID: $ROUTER_PID"
wait_for_server_ready --port "$PORT" --server-log "$ROUTER_LOG" --server-pid "$ROUTER_PID"
fi

if [ "${EVAL_ONLY}" = "true" ]; then
run_eval --port "$PORT"
else
build_replay_cmd "$RESULT_DIR"
run_agentic_replay_and_write_outputs "$RESULT_DIR"
fi
8 changes: 8 additions & 0 deletions configs/nvidia-master.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1781,15 +1781,23 @@
scenarios:
agentic-coding:
- dram-utilization: 0.80
search-space:
# Pure TP at low concurrency.
- { tp: 8, kv-offloading: none, conc-list: [1, 2, 4, 6, 8] }
# Pure TP GPU-resident + MTP (num_speculative_tokens=3)
- { tp: 8, kv-offloading: none, spec-decoding: mtp, conc-list: [1, 2, 4, 6, 8] }
- { tp: 8, kv-offloading: dram, kv-offload-backend: { name: vllm-simple, version: "904e4ec" }, conc-list: [8, 12, 16] }
# Pure TP SimpleCPU + MTP (num_speculative_tokens=3)
- { tp: 8, kv-offloading: dram, kv-offload-backend: { name: vllm-simple, version: "904e4ec" }, spec-decoding: mtp, conc-list: [8, 12, 16] }
# DEP
- { tp: 8, ep: 8, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: vllm-simple, version: "904e4ec" }, conc-list: [8, 16, 24, 32, 40, 48, 56, 64, 68, 72, 80], router: { name: vllm-router, version: "0.1.14" } }
# DEP SimpleCPU + MTP (num_speculative_tokens=3)
- { tp: 8, ep: 8, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: vllm-simple, version: "904e4ec" }, spec-decoding: mtp, conc-list: [8, 16, 24, 32, 40, 48, 56, 64, 68, 72, 80], router: { name: vllm-router, version: "0.1.14" } }
- { tp: 8, ep: 8, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: mooncake, version: "0.3.11.post1" }, conc-list: [12, 20, 28, 36, 44, 52, 60, 68, 76], router: { name: vllm-router, version: "0.1.14" } }
# DEP Mooncake + MTP (num_speculative_tokens=3)
- { tp: 8, ep: 8, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: mooncake, version: "0.3.11.post1" }, spec-decoding: mtp, conc-list: [12, 20, 28, 36, 44, 52, 60, 68, 76], router: { name: vllm-router, version: "0.1.14" } }

dsv4-fp4-b200-trt:

Check failure on line 1800 in configs/nvidia-master.yaml

View check run for this annotation

Claude / Claude Code Review

Missing perf-changelog.yaml entry for new MTP benchmark configs

The PR adds 6 new MTP search-space rows (and the new dsv4_fp4_b200_vllm_mtp.sh script) to dsv4-fp4-b200-vllm-agentic in configs/nvidia-master.yaml, but never appends the required entry to perf-changelog.yaml. Since .github/workflows/run-sweep.yml gates both its push and pull_request triggers on paths: perf-changelog.yaml, the benchmark sweep will never trigger for this PR, leaving the new MTP arms unbenchmarked and unvalidated by CI. Please append a config-keys: [dsv4-fp4-b200-vllm-agentic] entr
Comment on lines 959 to 990

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The PR adds 6 new MTP search-space rows (and the new dsv4_fp4_b200_vllm_mtp.sh script) to dsv4-fp4-b200-vllm-agentic in configs/nvidia-master.yaml, but never appends the required entry to perf-changelog.yaml. Since .github/workflows/run-sweep.yml gates both its push and pull_request triggers on paths: perf-changelog.yaml, the benchmark sweep will never trigger for this PR, leaving the new MTP arms unbenchmarked and unvalidated by CI. Please append a config-keys: [dsv4-fp4-b200-vllm-agentic] entry to perf-changelog.yaml before merging.

Extended reasoning...

What's missing

This PR introduces 6 new MTP-variant agentic search-space rows under dsv4-fp4-b200-vllm-agentic in configs/nvidia-master.yaml (MTP counterparts of the pure-TP, SimpleCPU-offload, and DEP/Mooncake tiers), plus the new benchmarks/single_node/agentic/dsv4_fp4_b200_vllm_mtp.sh script that actually runs them. However, the PR touches only three files — the new script, configs/nvidia-master.yaml, and utils/matrix_logic/validation.py — and never appends an entry to perf-changelog.yaml.

Why this matters (not just a doc nit)

AGENTS.md's "Adding a benchmark configuration" section is explicit: "Add entries to configs/nvidia-master.yaml ..., append to perf-changelog.yaml, then validate...". That alone would just be a process nit, except perf-changelog.yaml is not merely documentation — it is the mechanism that drives CI. .github/workflows/run-sweep.yml gates both its push (to main) and pull_request triggers on:

paths:
  - "perf-changelog.yaml"

I verified this directly in the workflow file (lines ~22-36). Since this PR never modifies perf-changelog.yaml, the check-changelog/validate-perf-changelog job and the downstream benchmark-sweep job never run for this PR at all — not "run and pass trivially," but never invoked in the first place.

Proof / concrete walkthrough

  1. I checked the tail of perf-changelog.yaml: the most recent entries reference PR [AgentX] Update vLLM DeepSeek-V4 B300 aggregate / 更新 vLLM DeepSeek-V4 B300 聚合配置 #2241 (dsv4-fp4-b300-vllm-agentic), Add Kimi K2.6 NVFP4 B300 EAGLE3 AgentX benchmark / 新增 Kimi K2.6 NVFP4 B300 EAGLE3 AgentX 基准测试 #2228, and [NV] Add GLM-5 NVFP4 GB200 disaggregated Dynamo TensorRT-LLM MTP benchmarks / [NV] 新增 GLM-5 NVFP4 GB200 分离式 Dynamo TensorRT-LLM MTP 基准测试 #1800 — there is no entry for this PR or its new MTP config keys.
  2. run-sweep.yml's on.pull_request.paths and on.push.paths both list only perf-changelog.yaml.
  3. GitHub Actions path-filtered triggers only fire the workflow when a commit in the PR touches a file matching that path. This PR's diff (dsv4_fp4_b200_vllm_mtp.sh, configs/nvidia-master.yaml, utils/matrix_logic/validation.py) matches none of those paths.
  4. Therefore run-sweep.yml does not trigger on this PR's pushes/synchronize events, so check_changelog/process_changelog.py never runs, never discovers the 6 new MTP rows (it discovers rows to benchmark by diffing perf-changelog.yaml between base and head), and the sweep infrastructure that would exercise dsv4_fp4_b200_vllm_mtp.sh never executes.
  5. Net effect: the new MTP arms — the entire purpose of this PR — go unbenchmarked and unvalidated by CI, silently. Nothing fails loudly; the workflow simply never starts.

Precedent

Every comparable recent PR in this repo followed the append-to-changelog step: #2224 (dsv4-fp4-b200-vllm-agentic), #2241 (dsv4-fp4-b300-vllm-agentic), #2228 (kimik2.5-fp4-b300-vllm-agentic-mtp), and #1800 (glm5-fp4-gb200-dynamo-trt-mtp) all appended a perf-changelog.yaml entry alongside their config changes. This PR breaks that established, repository-mandated pattern.

Fix

Append a new entry to the end of perf-changelog.yaml, e.g.:

- config-keys:
    - dsv4-fp4-b200-vllm-agentic
  description:
    - "Add MTP (num_speculative_tokens=3) variants of the pure-TP, SimpleCPU-offload, and DEP/Mooncake agentic tiers for B200 AgentX."
  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2259

This is not a stylistic ask — it is the mechanism by which the sweep workflow actually triggers, so omitting it defeats the PR's own verification.

image: ghcr.io#semianalysisai/trtllm-deepseek-v4:feat-deepseek_v4-c185066
model: deepseek-ai/DeepSeek-V4-Pro
model-prefix: dsv4
Expand Down
6 changes: 5 additions & 1 deletion utils/matrix_logic/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@ class SingleNodeAgenticMatrixEntry(BaseModel):
duration: int = Field(alias=Fields.DURATION.value)
exp_name: str = Field(alias=Fields.EXP_NAME.value)
scenario_type: str = Field(alias=Fields.SCENARIO_TYPE.value)
# Agentic eval rows (SWE-bench) carry run_eval/eval_only; benchmark rows omit
# them. Optional so both shapes validate and benchmark output is unchanged.
run_eval: Optional[bool] = Field(default=None, alias=Fields.RUN_EVAL.value)
eval_only: Optional[bool] = Field(default=None, alias=Fields.EVAL_ONLY.value)

@model_validator(mode='after')
def validate_kv_offload_fields(self):
Expand Down Expand Up @@ -889,7 +893,7 @@ class ChangelogMatrixEntry(BaseModel):
] = Field(default_factory=dict)
multi_node: dict[str, list[Union[MultiNodeMatrixEntry, MultiNodeAgenticMatrixEntry]]
] = Field(default_factory=dict)
evals: list[SingleNodeMatrixEntry] = Field(default_factory=list)
evals: list[Union[SingleNodeMatrixEntry, SingleNodeAgenticMatrixEntry]] = Field(default_factory=list)
multinode_evals: list[MultiNodeMatrixEntry] = Field(default_factory=list)
changelog_metadata: ChangelogMetadata

Expand Down