Skip to content

[None][test] Add TLLM_SPEC_DECODE_FORCE_NUM_ACCEPTED_TOKENS in Spec Decoding Perf Test - #14438

Merged
chenfeiz0326 merged 4 commits into
NVIDIA:mainfrom
chenfeiz0326:chenfeiz/add-al-for-spec-decoding-perf-test
May 31, 2026
Merged

[None][test] Add TLLM_SPEC_DECODE_FORCE_NUM_ACCEPTED_TOKENS in Spec Decoding Perf Test#14438
chenfeiz0326 merged 4 commits into
NVIDIA:mainfrom
chenfeiz0326:chenfeiz/add-al-for-spec-decoding-perf-test

Conversation

@chenfeiz0326

@chenfeiz0326 chenfeiz0326 commented May 22, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Code Change Summary

    ▎ 1. Always pass --ignore-eos, and stabilize accepted-token count via TLLM_SPEC_DECODE_FORCE_NUM_ACCEPTED_TOKENS (set per server_config in yaml; never computed in code).
    ▎ 2. New jenkins/scripts/perf/aggregated/submit.py for multi-node aggregated PerfSanity, plus three-way routing in L0_Test.groovy. Needed because trtllm-llmapi-launch dispatches rank 0 to pytest and other
    ▎ ranks to mgmn_worker_node — env vars must be set in the slurm script prefix to reach all ranks.
    ▎ 3. New d_al metric (acceptance length) for spec decoding tests, with l_force_num_accepted_tokens added as a baseline match key so different forced values match separately.
    ▎ 4. New d_mean_gen_worker_per_iter_device_step_time metric for gen_only tests; gen_only regression gates on this instead of throughput.
    ▎ 5. Per-test log isolation — all slurm logs land in <output_dir>/<test_case_name>/.
    ▎ 6. Yaml schema cleanup — agg yamls move env to per-server-config server_env_var; disagg adds spec-decode env to worker_env_var only.

Review Change Stack

Description

Test Coverage

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

@chenfeiz0326
chenfeiz0326 requested a review from a team as a code owner May 22, 2026 03:38
@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR extends a performance sanity test to measure and track speculative-decoding-only throughput metrics. It adds metric parsing for average decoded tokens per iteration, integrates it into categorization and regression logic, validates its presence for speculative-decoding clients, and conditionally uploads the metric to avoid OpenSearch baseline contamination.

Changes

Speculative-Decoding Metric Addition

Layer / File(s) Summary
Speculative-decoding metric constants and categorization
tests/integration/defs/perf/test_perf_sanity.py
New SPEC_DECODING_PERF_METRIC_LOG_QUERIES regex parses benchmark "Mean Avg Decoded Tokens per Iter" output into al metric; MAXIMIZE_METRICS and REGRESSION_METRICS now include d_al for speculative-decoding applicability.
Metric parsing integration and validation
tests/integration/defs/perf/test_perf_sanity.py
Benchmark output parsing merges speculative-decoding regex patterns to extract al when present; metric validation switches from count-based checks to missing-metric reporting and enforces al presence for spec-decoding clients with detailed error messaging.
Conditional metric upload for speculative-decoding
tests/integration/defs/perf/test_perf_sanity.py
Result upload logic conditionally writes d_al only for speculative-decoding test rows, omitting it from non-speculative rows to keep OpenSearch baselines aligned by population.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is unfilled boilerplate containing only the template with no actual content in Description or Test Coverage sections. Add a detailed description of the changes and explain why speculative-decoding AL metrics are being added. List the test coverage and complete the PR checklist with meaningful explanations.
Title check ⚠️ Warning The PR title mentions adding 'TLLM_SPEC_DECODE_FORCE_NUM_ACCEPTED_TOKENS', but the actual changes add support for 'al' metric parsing and 'd_al' regression metrics for speculative-decoding performance tests. The title does not accurately reflect the main changes. Update the title to reflect the actual changes, such as 'Add speculative-decoding performance metrics (al/d_al) to perf sanity test' or similar, to accurately describe the metric parsing and regression logic additions.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/integration/defs/perf/test_perf_sanity.py (1)

1-1: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the SPDX copyright year.

This file is modified in this 2026 PR, but Line 1 still ends at 2025.

Proposed fix
-# SPDX-FileCopyrightText: Copyright (c) 2022-2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-FileCopyrightText: Copyright (c) 2022-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.

As per coding guidelines, Include NVIDIA copyright header on ALL new files; update year on modified files.

🤖 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 `@tests/integration/defs/perf/test_perf_sanity.py` at line 1, The SPDX
copyright header at the top of test_perf_sanity.py still lists the end year as
2025; update the header comment to end with 2026 so the SPDX line matches the
file modification year (change "2022-2025" to "2022-2026" in the top-of-file
comment).
🤖 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.

Outside diff comments:
In `@tests/integration/defs/perf/test_perf_sanity.py`:
- Line 1: The SPDX copyright header at the top of test_perf_sanity.py still
lists the end year as 2025; update the header comment to end with 2026 so the
SPDX line matches the file modification year (change "2022-2025" to "2022-2026"
in the top-of-file comment).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e7df7015-259b-46f2-a83b-09270d749788

📥 Commits

Reviewing files that changed from the base of the PR and between 79ede08 and 21b09b7.

📒 Files selected for processing (1)
  • tests/integration/defs/perf/test_perf_sanity.py

@chenfeiz0326
chenfeiz0326 force-pushed the chenfeiz/add-al-for-spec-decoding-perf-test branch from c002d33 to 33ec282 Compare May 27, 2026 07:04
@chenfeiz0326 chenfeiz0326 changed the title [None][test] Add AL and Upload to Perf Database [None][test] Add Metrics for Spec decoding and Gen only May 27, 2026
@chenfeiz0326

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast --stage-list "DGX_B200-16_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE1-GPU8-Post-Merge-1,GB200-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-2"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50517 [ run ] triggered by Bot. Commit: 5dda089 Link to invocation

@fredricz-20070104 fredricz-20070104 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approve

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50517 [ run ] completed with state SUCCESS. Commit: 5dda089
/LLM/main/L0_MergeRequest_PR pipeline #40025 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@chenfeiz0326
chenfeiz0326 requested a review from a team May 29, 2026 04:18
@chenfeiz0326
chenfeiz0326 requested a review from a team as a code owner May 29, 2026 04:18
@chenfeiz0326
chenfeiz0326 force-pushed the chenfeiz/add-al-for-spec-decoding-perf-test branch from 561f140 to 4eb225f Compare May 29, 2026 06:18
@chenfeiz0326 chenfeiz0326 changed the title [None][test] Add Metrics for Spec decoding and Gen only [None][test] Add TLLM_SPEC_DECODE_FORCE_NUM_ACCEPTED_TOKENS in Spec Decoding Perf Test May 29, 2026
@chenfeiz0326

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast --stage-list "GB300-8_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE1-GPU4-Post-Merge-4,GB300-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-2,GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-2"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51038 [ run ] triggered by Bot. Commit: 672389a Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51038 [ run ] completed with state SUCCESS. Commit: 672389a
/LLM/main/L0_MergeRequest_PR pipeline #40485 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@chenfeiz0326

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast --stage-list "GB300-8_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE1-GPU4-Post-Merge-4,GB300-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-2"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51057 [ run ] triggered by Bot. Commit: feb2679 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51057 [ run ] completed with state SUCCESS. Commit: feb2679
/LLM/main/L0_MergeRequest_PR pipeline #40503 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@chenfeiz0326
chenfeiz0326 force-pushed the chenfeiz/add-al-for-spec-decoding-perf-test branch from feb2679 to bd8fa9e Compare May 30, 2026 03:47
@chenfeiz0326

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast --stage-list "GB300-8_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE1-GPU4-Post-Merge-4,GB300-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-2"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51152 [ run ] triggered by Bot. Commit: bd8fa9e Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51152 [ run ] completed with state FAILURE. Commit: bd8fa9e
/LLM/main/L0_MergeRequest_PR pipeline #40586 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@chenfeiz0326

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast --stage-list "GB300-8_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE1-GPU4-Post-Merge-4,GB300-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-2"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51162 [ run ] triggered by Bot. Commit: bd8fa9e Link to invocation

Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
@chenfeiz0326
chenfeiz0326 force-pushed the chenfeiz/add-al-for-spec-decoding-perf-test branch from bd8fa9e to 61f23f0 Compare May 30, 2026 06:46
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51162 [ run ] completed with state SUCCESS. Commit: bd8fa9e
/LLM/main/L0_MergeRequest_PR pipeline #40596 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
@chenfeiz0326

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast --stage-list "GB300-8_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE1-GPU4-Post-Merge-4,GB300-8_GPUs-2_Nodes-PyTorch-PerfSanity-Node2-GPU8-Post-Merge-2"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51174 [ run ] triggered by Bot. Commit: 6225e92 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51174 [ run ] completed with state FAILURE. Commit: 6225e92
/LLM/main/L0_MergeRequest_PR pipeline #40606 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
@chenfeiz0326

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast --stage-list "GB200-8_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE1-GPU4-Post-Merge-1,GB200-8_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE1-GPU4-Post-Merge-2,GB200-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-6,GB200-8_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU1-GEN1-NODE1-GPU2-Post-Merge-3,GB200-8_GPUs-2_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU1-GEN1-NODE1-GPU2-Post-Merge-4,GB200-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-8,GB200-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU1-GEN1-NODE2-GPU8-Post-Merge-2"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51186 [ run ] triggered by Bot. Commit: 7b34c90 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51186 [ run ] completed with state FAILURE. Commit: 7b34c90
/LLM/main/L0_MergeRequest_PR pipeline #40616 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
@chenfeiz0326

Copy link
Copy Markdown
Collaborator Author

/bot skip --comment "Only add new perf tests, no need to run the whole CI pipeline"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51192 [ skip ] triggered by Bot. Commit: 35148d2 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51192 [ skip ] completed with state SUCCESS. Commit: 35148d2
Skipping testing for commit 35148d2

Link to invocation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants