[None][test] Pair gen_only with gen_only_no_context as a control group in QA multinode perf list - #19324
Open
fredricz-20070104 wants to merge 3 commits into
Conversation
perf-sanity gained a gen_only_no_context benchmark mode (NVIDIA#19107): the gen worker fabricates its own KV blocks, so no ctx fleet is allocated and the decode loop is measured on the gen worker alone. Add four such ids to the QA weekly multi-node perf list as a CONTROL GROUP for the gen_only ids that are already there -- not as a replacement for them. Nothing is removed. All nine disagg-gen_only- ids stay exactly as they are; each of the four new aggr-gen_only_no_context- ids pairs with one of them over the SAME config yaml, two per GPU: gb200_gpt-oss-120b con4_ctx1_tp1_gen1_tp4 gb200_deepseek-r1 con1_ctx1_dep4_gen1_tep8 gb300_glm-5 con1024_ctx1_dep2_gen1_dep8 gb300_kimi-k25 con4_ctx1_dep4_gen1_tep8 Why a pair and not a substitution: NVIDIA#19107 measured a per-case gap between the two modes spanning -37%..+13%, so until the two decode metrics are shown to track each other per case, neither can stand in for the other. A case moved from gen_only to gen_only_no_context has one arm, not two, and produces no correlation data at all -- it stops reporting the very metric you would correlate against. Why these four: each one's disagg-gen_only- twin already has decode-metric history in the multi-node perf DB, so the paired series starts against a measured baseline instead of against nothing. Point counts over 1.3.0rc1..rc27, on the GPU each config's prefix names: 16, 12, 11 and 12. Two of the four (gpt-oss, deepseek-r1) were characterised in both arms by NVIDIA#19107 (-8.58% with 1.26% spread, -6.21% with 1.12%), which anchors the comparison; the other two were not (glm-5 measured but 12.68% peak-to-peak, kimi never measured in the no-context arm), which is why a longer paired run is needed. Deliberately excluded: the con1024 kimi ids on both GPUs, which have 0 history points -- nothing to pair against -- and gb200_wideep, whose 8-node gen worker is the regime where NVIDIA#19107 saw a 20.4% Slurm-startup retry rate against 0% single-node. No config yaml change, by design: the mode reads the existing disaggregated yaml and overrides num_ctx_servers / num_gen_servers to (0, 1) itself, which is what keeps each pair two ids over one file rather than two files that silently drift. aggr-, not disagg-: the prefix names the launch path, and this mode runs one pytest owning the gen worker, proxy and client, the same single-pytest path ctx_only already takes out of a disaggregated yaml. Cache-transceiver coverage is unaffected. All four configs also run disagg-e2e- in this same list, and e2e is what exercises the KV transfer between workers; what gen_only adds over gen_only_no_context is real KV arriving at the gen worker, which those e2e ids already cover. The new ids start their own baseline lineage -- s_benchmark_mode is an OpenSearch match key -- so their decode metric is un-gated until history accumulates. That is a cost of adding an arm, not of moving a case: nothing above loses its lineage, because nothing above was removed. Depends on NVIDIA#19107, which adds the mode to the perf-sanity system. Do not merge this before it: without NVIDIA#19107 these four ids do not resolve to a runnable benchmark mode. Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com> Co-Authored-By: Claude <noreply@anthropic.com>
fredricz-20070104
force-pushed
the
user/fredricz/qa-multinode-gen-only-no-context-pair
branch
from
September 17, 2026 05:36
e5deca6 to
97ab10e
Compare
Collaborator
Author
|
/bot skip --comment "skip test as just modify some test configs, only run in qa side" |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review. WalkthroughThe multi-node test list documents ChangesMulti-node performance coverage
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other Merge Risk: ⚪ Minimal · up to The PR adds documented QA control cases without an identified production or test-harness failure risk. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Collaborator
|
PR_Github #74022 [ skip ] triggered by Bot. Commit: |
ruodil
approved these changes
Sep 17, 2026
|
Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements. |
Collaborator
|
PR_Github #74022 [ skip ] completed with state |
yufeiwu-nv
removed their request for review
September 17, 2026 23:31
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
perf-sanity gained a
gen_only_no_contextbenchmark mode in #19107: the gen workerfabricates its own KV blocks, so no ctx fleet is allocated and the decode loop is
measured on the gen worker alone. This adds four
aggr-gen_only_no_context-ids tothe QA weekly multi-node perf list (
tests/integration/test_lists/qa/llm_perf_multinode.txt)as a control group for the
gen_onlyids already there — not as a replacement.Nothing is removed. All nine
disagg-gen_only-ids stay exactly as they are;each new id pairs with one of them over the same config yaml, two per GPU:
disagg-gen_only-aggr-gen_only_no_context-gen_onlyhistory pointsgb200_gpt-oss-120b-fp4_8k1k_con4_ctx1_tp1_gen1_tp4_eplb0_mtp0_ccbgb200_deepseek-r1-fp4_8k1k_con1_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccbgb300_glm-5-fp4_8k1k_con1024_ctx1_dep2_gen1_dep8_eplb256_mtp1_ccbgb300_kimi-k25-thinking-fp4_8k1k_con4_ctx1_dep4_gen1_tep8_eplb0_mtp3_ccbWhy a pair, not a substitution. #19107 measured a per-case gap between the two
modes spanning -37%..+13%. Until the two decode metrics are shown to track each
other per case, neither can stand in for the other — and a case moved from
gen_onlytogen_only_no_contexthas one arm, not two, so it produces nocorrelation data at all: it stops reporting the very metric you would correlate
against. Each config's
supported_gpusroutes it to exactly one GPU chain, so thepaired run is 4 cases on GB200-LYRIS and 4 on GB300-LYRIS.
Why these four. Each one's
disagg-gen_only-twin already has decode-metric(
prev_device_step_time) history in the multi-node perf DB, so the paired seriesstarts against a measured baseline instead of against nothing — point counts over
1.3.0rc1..rc27above. Two of the four (gpt-oss, deepseek-r1) were characterised inboth arms by #19107 (-8.58% with 1.26% spread, -6.21% with 1.12%), which anchors the
comparison; the other two were not (glm-5 measured but 12.68% peak-to-peak, kimi
never measured in the no-context arm), which is why a longer paired run is needed at
all. Deliberately excluded: the
con1024kimi ids on both GPUs (0 history points —nothing to pair against) and
gb200_wideep, whose 8-node gen worker is the regimewhere #19107 saw a 20.4% Slurm-startup retry rate against 0% single-node.
No config yaml change, by design. The mode reads the existing disaggregated yaml
and overrides
num_ctx_servers/num_gen_serversto(0, 1)itself(
gen_only_no_context_server_counts), which is what keeps each pair two ids over onefile rather than two files that silently drift.
aggr-, notdisagg-: the prefixnames the launch path, not the topology — this mode runs one pytest owning the gen
worker, proxy and client, the same single-pytest path
ctx_onlyalready takes out ofa disaggregated yaml.
Baselines do not carry over between modes.
s_benchmark_modeis an OpenSearchmatch key, so each new id starts its own baseline lineage and its decode metric is
un-gated until history accumulates. That is a cost of adding an arm, not of moving
a case — nothing above loses its lineage, because nothing was removed.
Please do not merge this before #19107. That PR is what teaches the perf-sanity
system the
gen_only_no_contextmode; without it these four ids do not resolve to arunnable benchmark mode. The rationale numbers quoted above are also from #19107's
measurements.
Test Coverage
This PR is test-list content — it adds four perf-sanity test ids and changes no
product or test code:
Cache-transceiver coverage is unaffected: all four configs also run
disagg-e2e-inthis same list, and
e2eis what exercises the KV transfer between workers. Whatgen_onlyadds overgen_only_no_contextis real KV arriving at the gen worker,which those
e2eids already cover for these configs.The correlation data itself is being collected outside the weekly QA pipeline, by a
dedicated daily internal GitLab schedule (
dlswqa/trtllm-cischedule 25592,"9. TRTLLM Gen-Only No-Context Correlation Pipeline") that carries these 8 ids inline
on both GPUs, so nothing here waits on the QA schedule cadence to reach >10 paired
points per case.
PR Checklist
🤖 Generated with Claude Code
Dev Engineer Review
The change adds four
aggr-gen_only_no_context-benchmark IDs. It leaves existingdisagg-gen_only-entries and configuration YAML files unchanged. The new entries use no context fleet configuration and depend ongen_only_no_contextsupport from#19107. No source-code or API changes are included.QA Engineer Review
tests/integration/test_lists/qa/llm_perf_multinode.txtadds four manual-QA entries for GB200 GPT-OSS-120B, GB200 DeepSeek-R1, GB300 GLM-5, and GB300 Kimi-K2.5-Thinking. The entries pair with existingdisagg-gen_only-configurations, while existingdisagg-e2e-entries retain KV-transfer coverage. The entries are in the appropriateqa/manual-QA list. No test execution results are supplied; CI was intentionally skipped. Coverage verdict: sufficient.Per-File QA Perspective
tests/integration/test_lists/qa/llm_perf_multinode.txt: Adds four manual-QA IDs for generation without a context fleet. Verify that each ID resolves to the intended configuration and produces an independent baseline lineage.