fix(mocker): align replay ITL and per-user throughput with AIPerf semantics - #12883
fix(mocker): align replay ITL and per-user throughput with AIPerf semantics#12883davilu-nvidia wants to merge 1 commit into
Conversation
…antics
The replay report prints an AIPerf-style table, but its Inter Token
Latency and Output Token Throughput Per User rows were computed over
individual consecutive-token gaps pooled across requests, while AIPerf
defines both metrics per request:
itl = (request_latency - ttft) / (output_sequence_length - 1)
otpu = 1 / itl
Comparing a DynoSim report against a real AIPerf run therefore
overstated the simulation error dramatically. In a GB200 validation run
(Qwen3-32B TP2 x4 workers, Mooncake FAST25 conversation trace via the
dynamo frontend + SGLang workers, measured with AIPerf 0.9.0), the
per-gap ITL percentiles made the sim look 60-90% optimistic at
p90/p99, while the request-level comparison shows 31/45%:
silicon sim (per-gap) sim (request-level)
ITL p50 (ms) 12.1 8.0 9.6
ITL p90 (ms) 21.4 8.6 14.8
ITL p99 (ms) 237.0 24.2 129.8
itl_* and output_token_throughput_per_user_* now follow the AIPerf
definitions. The raw per-gap distribution is still valuable - its tail
is the only place engine stalls such as chunked-prefill interference
are visible before request-level averaging smooths them out - so it is
kept under new token_gap_* summary fields and a Token Gap table row.
Signed-off-by: davilu <davilu@nvidia.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
|
👋 Hi davilu-nvidia! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
github-actions
Bot
added
external-contribution
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 New token_gap fields escape the canonical validation and handoff-conformance checks
validate_report_finite checks ttft/ttst/tpot/itl/e2e/output_token_throughput_per_user (lib/mocker/src/replay/offline/canonical.rs:464-473) and the offline handoff conformance comparator compares latency.itl.distribution plus latency.itl.max_ms (lib/mocker/src/replay/offline/single.rs:717-753). Both are now looking at the request-level distribution, which is a copy of tpot; the per-gap distribution moved to token_gap and is compared/validated nowhere. Net effect: the serialized *_token_gap_ms fields are unvalidated, and the single-worker vs. generic-runtime conformance check silently lost its only per-gap-timing assertion (a divergence in individual token spacing that preserves the request average would no longer be caught).
(Refers to lines 1290-1291)
Was this helpful? React with 👍 or 👎 to provide feedback.
| tpot: build_distribution_stats(tpots.clone()), | ||
| itl: { | ||
| let distribution = build_distribution_stats(tpots); | ||
| TraceInterTokenLatencyStats { | ||
| max_ms: distribution.max_ms, | ||
| distribution, | ||
| } | ||
| }, |
There was a problem hiding this comment.
🔍 ITL now duplicates TPOT field-for-field
Because e2e_ms is computed as last_token_ms - arrival_time_ms and ttft_ms as first_token_ms - arrival_time_ms (lib/mocker/src/replay/collector.rs:1255-1256), AIPerf's (request_latency - ttft) / (osl - 1) is algebraically identical to TraceRequestStats::mean_tpot_ms (lib/mocker/src/replay/collector.rs:721-730). The new itl distribution is therefore built from exactly the same tpots vector as tpot, so every *_itl_ms field in the report is now bit-identical to the corresponding *_tpot_ms field, and the extra tpots.clone() exists solely to feed two identical distributions. This is functionally fine but worth confirming: consumers that used to read both rows for distinct information now get duplicate data, and the clone could be avoided by computing the stats once and copying the struct.
Was this helpful? React with 👍 or 👎 to provide feedback.
| output_token_throughput_per_user: build_distribution_stats( | ||
| output_tokens_per_s_per_user, | ||
| ), |
There was a problem hiding this comment.
🔍 Downstream Spica scoring docs/comments still describe the old per-gap definition
mean_output_token_throughput_per_user is the objective metric for OptimizationTarget.THROUGHPUT_PER_USER (aisimulate/src/aisimulate/spica/score.py:82-84), whose comment states it is the "mean of per-token-gap 1000/itl"; the same wording appears in docs/fern/pages/developer-guide/knowledge-base/modular-components/ai-simulate-experimental/spica-experimental/optimization-goals.md:50. After this change the value is the mean of per-request 1000/ITL, so both the comment and the doc are stale, and previously recorded sweep results are no longer comparable with new ones for this target.
Was this helpful? React with 👍 or 👎 to provide feedback.
WalkthroughReplay latency reporting now separates request-level ITL from pooled token gaps. It serializes token-gap statistics, computes per-user throughput from request-level TPOT, updates report display and documentation, and revises tests. ChangesReplay latency metrics
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/mocker/src/replay/collector.rs`:
- Around line 1275-1277: Define a single AIPerf-compatible contract for zero ITL
and apply it in the throughput calculation near output_tokens_per_s_per_user:
ensure every request receives the documented throughput value when tpot_ms is
0.0, and add a same-timestamp-token test covering that behavior in
lib/mocker/src/replay/collector.rs (anchor lines 1275-1277). Update
docs/fern/pages/reference/components/dynosim-replay-cli-reference.mdx (lines
352-357) to precisely describe the selected zero-ITL behavior; both sites must
remain consistent.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fea0473f-6b9f-4408-9511-b57c48528a3e
📒 Files selected for processing (4)
components/src/dynamo/replay/reporting.pydocs/fern/pages/reference/components/dynosim-replay-cli-reference.mdxlib/mocker/src/replay/collector.rslib/mocker/src/replay/mod.rs
| if tpot_ms > 0.0 { | ||
| output_tokens_per_s_per_user.push(1000.0 / tpot_ms); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Define one zero-ITL throughput contract across collection and documentation.
A request can have zero ITL when multiple tokens receive the same completion timestamp. The current collector excludes that request from per-user throughput, while the documentation promises a throughput value per request.
lib/mocker/src/replay/collector.rs#L1275-L1277: define and implement the AIPerf-compatible behavior fortpot_ms == 0.0, then add a same-timestamp token test.docs/fern/pages/reference/components/dynosim-replay-cli-reference.mdx#L352-L357: document the selected zero-ITL behavior precisely.
📍 Affects 2 files
lib/mocker/src/replay/collector.rs#L1275-L1277(this comment)docs/fern/pages/reference/components/dynosim-replay-cli-reference.mdx#L352-L357
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/mocker/src/replay/collector.rs` around lines 1275 - 1277, Define a single
AIPerf-compatible contract for zero ITL and apply it in the throughput
calculation near output_tokens_per_s_per_user: ensure every request receives the
documented throughput value when tpot_ms is 0.0, and add a same-timestamp-token
test covering that behavior in lib/mocker/src/replay/collector.rs (anchor lines
1275-1277). Update
docs/fern/pages/reference/components/dynosim-replay-cli-reference.mdx (lines
352-357) to precisely describe the selected zero-ITL behavior; both sites must
remain consistent.
|
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
1 participant
Overview
The replay report prints an AIPerf-style metrics table, but its Inter Token Latency and Output Token Throughput Per User rows were computed over individual consecutive-token gaps pooled across all requests, while AIPerf defines both metrics per request:
Since DynoSim's headline use case is comparing a simulated report against a real AIPerf run field-for-field, the like-named metrics should use the same definition. With the per-gap definition, percentile rows are dominated by clean decode steps: rare-but-large scheduler stalls (e.g. a chunked prefill blocking decode for ~200 ms) sit just above p99 and disappear from the table, while on the AIPerf side each such stall is smeared into every affected request's average and shifts the whole distribution.
What changed
itl_*summary fields and the "Inter Token Latency" table row now follow AIPerf's request-level definition (identical to the existing per-requestitl_msin--per-request-jsonl)output_token_throughput_per_user_*now follows AIPerf (1 / itlper request)token_gap_*summary fields and a "Token Gap (ms)" table row — its tail is the only place engine-level stalls such as chunked-prefill interference remain visible before request-level averaging smooths them outtest_replay_itl_uses_per_token_gapsrenamed totest_replay_itl_matches_aiperf_request_semantics, covering both metricsValidation against silicon
Setup: Qwen3-32B (bf16), 4x SGLang TP2 workers on GB200 behind
dynamo.frontend --router-mode kv(this repo @ main), replaying the public Mooncake FAST25conversation_trace.jsonl(first 20 min, filtered to input+output <= 40704, 3440 requests,ignore_eos) with AIPerf 0.9.0 in fixed-schedule mode; DynoSim replaying the identical trace with--router-mode kv_router --num-workers 4and AIC gb200/sglang timing, KV capacity pinned to the measuredmax_total_num_tokens.The old rows made the simulator look 60-90% optimistic on decode latency; the apples-to-apples comparison shows the actual model error is 21-45%, which matches the physical accounting (engine logs show a 15.7% chunked-prefill duty cycle; the mocker reproduces ~104 s of blocked stream-time vs ~145 s on silicon, plus AIC's GB200 decode entries running ~7% fast).
Breaking change note
mean/median/p*/max_itl_msand*_output_token_throughput_per_userchange meaning (per-gap -> per-request). Consumers that want the old data should read the new*_token_gap_msfields.Summary by CodeRabbit
New Features
Documentation