Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[submodule "utils/aiperf"]
path = utils/aiperf
url = https://github.com/SemiAnalysisAI/aiperf.git
branch = cquil11/aiperf-agentx-v1.0
branch = cquil11/deterministic-agentic-warmup-count-main
16 changes: 10 additions & 6 deletions benchmarks/benchmark_lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1744,14 +1744,13 @@ build_replay_cmd() {
# utils/aiperf/docs/tutorials/agentx-mvp.md.
local result_dir="$1"
local duration="$DURATION"
local cache_warmup_duration="${AIPERF_AGENTIC_CACHE_WARMUP_DURATION:-600}"
local warmup_requests_per_lane="${AIPERF_WARMUP_REQUESTS_PER_LANE:-10}"
Comment on lines 1744 to +1747

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 AIPerf warmup rename (AIPERF_AGENTIC_CACHE_WARMUP_DURATIONAIPERF_WARMUP_REQUESTS_PER_LANE) in build_replay_cmd() isn't propagated to its consumers: the 4 GB300 DeepSeek-V4 agentic recipe YAMLs (disagg-gb300-1p1d-dep4-dep8-c128-mtp-agentic.yaml:201, agg-gb300-tp8-mtp-agentic.yaml:147, agg-gb300-tp4-mtp-agentic.yaml:144, disagg-gb300-1p1d-dep8-dep8-c384-mtp-agentic.yaml:202) still set the now-dead AIPERF_AGENTIC_CACHE_WARMUP_DURATION: "600", and benchmarks/multi_node/amd_utils/server_sglang.sh:989 still forwards that dead var into the client container instead of the new AIPERF_WARMUP_REQUESTS_PER_LANE. Nothing crashes since those recipes just fall back to the new default (10 requests/lane), but the stale env vars are misleading and worth cleaning up, and AMD multi-node runs currently have no way to override the new knob via that allowlist.

Extended reasoning...

build_replay_cmd() in benchmarks/benchmark_lib.sh (used by every agentic recipe, including the 4 GB300 DeepSeek-V4 ones despite the PR's 'B300-only' framing) is changed to read AIPERF_WARMUP_REQUESTS_PER_LANE (default 10) and emit --warmup-requests-per-lane, replacing the old AIPERF_AGENTIC_CACHE_WARMUP_DURATION (default 600) / --agentic-cache-warmup-duration path entirely. That is exactly the intended behavior change described in the PR summary, so this itself isn't a bug — but two consumers of the old var were left un-migrated.

Step-by-step proof:

  1. Before this PR, disagg-gb300-1p1d-dep4-dep8-c128-mtp-agentic.yaml:201 (and the other 3 GB300 agentic recipes) set AIPERF_AGENTIC_CACHE_WARMUP_DURATION: "600" in their env, which build_replay_cmd() read via ${AIPERF_AGENTIC_CACHE_WARMUP_DURATION:-600} and passed as --agentic-cache-warmup-duration 600.
  2. After this PR, build_replay_cmd() no longer references that var at all — grep confirms zero remaining reads in benchmark_lib.sh. The recipe's AIPERF_AGENTIC_CACHE_WARMUP_DURATION: "600" setting becomes a silent no-op: it's still exported into the environment but nothing consumes it.
  3. Instead, build_replay_cmd() now reads AIPERF_WARMUP_REQUESTS_PER_LANE, which none of these 4 YAMLs set, so they silently pick up the new default of 10 requests/lane via ${AIPERF_WARMUP_REQUESTS_PER_LANE:-10}.
  4. Separately, benchmarks/multi_node/amd_utils/server_sglang.sh:989 forwards AIPERF_AGENTIC_CACHE_WARMUP_DURATION (now dead) into the client container's env allowlist, but does not forward AIPERF_WARMUP_REQUESTS_PER_LANE. So even if an AMD multi-node recipe wanted to override the new per-lane count, there's currently no path for that override to reach the client container.

Existing code doesn't catch this because there's no validation that env vars referenced in recipe YAMLs are actually consumed anywhere — a renamed/removed var is simply silently ignored rather than erroring, and the env-forward allowlist is a static list that has to be manually kept in sync with whatever knobs build_replay_cmd() currently exposes.

Impact is limited: because the old default was also 600s (i.e., these recipes were never overriding the default, just re-stating it), no customized behavior is actually being lost — they're just moving to the new mechanism's default like every other agentic recipe would after this PR merges, which is the PR's intended outcome. Nothing errors or crashes. The concrete cost is (a) 4 lines of dead/misleading config in production recipe YAMLs that should be updated or removed, and (b) a gap in the AMD env-forward allowlist that blocks overriding the new knob for multi-node AMD runs until it's added.

Fix: replace AIPERF_AGENTIC_CACHE_WARMUP_DURATION: "600" with the equivalent AIPERF_WARMUP_REQUESTS_PER_LANE setting (if a non-default value is actually desired) or drop the stale key entirely in the 4 YAMLs, and swap the forwarded var name at server_sglang.sh:989 from AIPERF_AGENTIC_CACHE_WARMUP_DURATION to AIPERF_WARMUP_REQUESTS_PER_LANE.


# Fast mode is an e2e-only feedback preset used before canonical one-hour
# sweeps. AIPerf already exposes both controls, so no AIPerf patch is
# required.
if [[ "${AIPERF_EXPERIMENTAL_FAST:-0}" == "1" ]]; then
duration=1200
cache_warmup_duration=300
fi

export AIPERF_DATASET_WEKA_LIVE_ASSISTANT_RESPONSES="${AIPERF_DATASET_WEKA_LIVE_ASSISTANT_RESPONSES:-0}"
Comment on lines 1744 to 1756

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.

🟡 Fast mode (AIPERF_EXPERIMENTAL_FAST=1) previously halved cache warmup along with duration, but this PR removes that override without adding an equivalent reduction for warmup_requests_per_lane — fast mode now only shortens --benchmark-duration while warmup stays at the canonical 10 requests/lane. This also leaves AGENTS.md:122 stale, since it still describes the old duration-based warmup ('5-minute cache warmup ... canonical AgentX timing remains a 10-minute cache warmup') that no longer exists after this change.

Extended reasoning...

What changed and what broke: build_replay_cmd() in benchmarks/benchmark_lib.sh swaps the duration-based AgentX cache warmup (--agentic-cache-warmup-duration) for a deterministic request-count warmup (--warmup-requests-per-lane). Before this PR, the AIPERF_EXPERIMENTAL_FAST=1 branch shortened both knobs for fast e2e feedback runs:

if [[ "${AIPERF_EXPERIMENTAL_FAST:-0}" == "1" ]]; then
    duration=1200
    cache_warmup_duration=300   # <-- halved from the 600s default
fi

After the PR, only duration is shortened; the cache_warmup_duration=300 line was deleted with no analogous adjustment to warmup_requests_per_lane:

if [[ "${AIPERF_EXPERIMENTAL_FAST:-0}" == "1" ]]; then
    duration=1200
fi

Step-by-step proof:

  1. A user dispatches an AgentX preflight with agentx-fast=true (per AGENTS.md:122), which sets AIPERF_EXPERIMENTAL_FAST=1.
  2. build_replay_cmd() runs; duration becomes 1200 (20 min), but warmup_requests_per_lane stays at its default of 10 (AIPERF_WARMUP_REQUESTS_PER_LANE:-10}), identical to a canonical one-hour run.
  3. The emitted aiperf command carries --benchmark-duration 1200 --warmup-requests-per-lane 10 — the same warmup cost as a full canonical sweep, just with a shorter profiling window afterward.
  4. Anyone relying on fast mode to also cut warmup time (as it did pre-PR) gets a smaller-than-expected speedup, and doesn't discover this unless they read the new code — nothing signals the behavior change.

Why nothing catches this: there's no test or assertion tying fast-mode env vars to a proportional warmup reduction; it was purely convention in the old code. bash -n and git diff --check (the PR's stated validation) only check syntax, not behavior parity with the prior fast-mode contract.

Documentation impact: AGENTS.md:122 still reads: "This selects a 5-minute cache warmup and 20-minute profile for every AgentX job... canonical AgentX timing remains a 10-minute cache warmup and 1-hour profile." That text describes the removed --agentic-cache-warmup-duration mechanism (5 min / 10 min splits) which no longer exists in either mode — warmup is now request-count based via --warmup-requests-per-lane, and both fast and canonical runs use the same count (10 requests/lane). This line should be rewritten to describe the new request-count semantics and either state that fast mode currently does not shorten warmup, or add a corresponding fast-mode override for warmup_requests_per_lane if a shorter warmup is still desired.

Fix options: (a) add an explicit fast-mode override, e.g. warmup_requests_per_lane=... inside the AIPERF_EXPERIMENTAL_FAST branch to restore proportional warmup savings, and/or (b) update AGENTS.md:122 to reflect that warmup is now request-count based rather than duration based, removing the now-inaccurate "5-minute cache warmup" / "10-minute cache warmup" wording.

This is a defensible, low-risk change on its own (10 request-per-lane warmup with no idle delay is already fast), so nothing crashes or produces incorrect benchmark data — it's a silent behavior change to an experimental fast-mode preset plus a stale doc, not a functional regression.

Expand All @@ -1765,6 +1764,9 @@ build_replay_cmd() {
# aiperf validates that SERVICE_PROFILE_CONFIGURE_TIMEOUT >=
# DATASET_CONFIGURATION_TIMEOUT at startup. Bump it in lockstep.
export AIPERF_SERVICE_PROFILE_CONFIGURE_TIMEOUT=1800
# Headless realtime metrics are opt-in on current AIPerf main. Enable the
# rolling TTFT/ITL/throughput block and emit it every 30 seconds.
export AIPERF_UI_REALTIME_METRICS_ENABLED=true
REPLAY_CMD="$AIPERF_CLI profile --scenario inferencex-agentx-mvp"
REPLAY_CMD+=" --url http://localhost:$PORT"
REPLAY_CMD+=" --endpoint /v1/chat/completions"
Expand All @@ -1773,6 +1775,7 @@ build_replay_cmd() {
REPLAY_CMD+=" --model $MODEL"
REPLAY_CMD+=" --concurrency $CONC"
REPLAY_CMD+=" --benchmark-duration $duration"
REPLAY_CMD+=" --stats-interval 30"
REPLAY_CMD+=" --random-seed 42"
# Fail runs once more than 10% of requests error. This keeps known
# transient low-rate failures from killing long sweeps while still
Expand All @@ -1784,10 +1787,11 @@ build_replay_cmd() {
# least one profile turn after warmup.
REPLAY_CMD+=" --trajectory-start-min-ratio 0.25"
REPLAY_CMD+=" --trajectory-start-max-ratio 0.75"
# After the normal t* snapshot warmup, continue those exact trajectories
# with one-token outputs and no idle delays. Profiling begins only after
# those requests drain and resumes from the resulting live state.
REPLAY_CMD+=" --agentic-cache-warmup-duration $cache_warmup_duration"
# After the normal t* snapshot warmup, advance every trajectory lane by
# the same deterministic number of one-token requests with no idle delay.
# Profiling begins after those requests drain and resumes from the
# resulting live state.
REPLAY_CMD+=" --warmup-requests-per-lane $warmup_requests_per_lane"
# Give long-context warmup requests up to 30 minutes to drain before
# declaring warmup failed. Recipes whose saturation arms carry a larger
# in-flight working set may override via AGENTIC_WARMUP_GRACE_PERIOD
Expand Down
6 changes: 6 additions & 0 deletions perf-changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5191,3 +5191,9 @@
- "Agentic experiment Variant K (probe, of the #2374 Variant G): REMOVES the kimi-k3-container-deps.sh in-container patch (setup_script, script file, and launcher copy) to verify whether the mamba_hybrid index_fill_ dtype patch is still required by the current vllm/vllm-openai:kimi-k3 image — expected to fail at the first decode step if it is"
- "Recipe: benchmarks/multi_node/srt-slurm-recipes/vllm/kimi-k3/agentic/agg-b200-tp8pp2-agentic.yaml on the cluster:b200-dgxc pool"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2391

- config-keys:
- kimik3-fp4-b300-vllm-agentic
description:
- "Test deterministic AIPerf warmup on Kimi K3 B300 with 10 requests per lane."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2408
2 changes: 1 addition & 1 deletion utils/aiperf
Submodule aiperf updated 1777 files
Loading