Skip to content

bench: add topology KV candidate schedule benchmark#1160

Open
teerthsharma wants to merge 28 commits into
alibaba:mainfrom
teerthsharma:codex/topology-kv-candidate-schedule
Open

bench: add topology KV candidate schedule benchmark#1160
teerthsharma wants to merge 28 commits into
alibaba:mainfrom
teerthsharma:codex/topology-kv-candidate-schedule

Conversation

@teerthsharma

@teerthsharma teerthsharma commented Jul 6, 2026

Copy link
Copy Markdown

Summary

This PR adds an opt-in benchmark for evaluating topology-derived KV candidate schedules during long-context decode attention.

The benchmark:

  • builds causal candidate block rows from K-cache block centroids,
  • combines attention-sink blocks, recent local blocks, and high-drift witness blocks,
  • expands the selected block row into token indices,
  • compares dense PyTorch SDPA against SDPA over gathered candidate K/V tokens,
  • stays fully isolated under benchmark/ with no serving-path behavior changes.

This is a screening benchmark only. It does not claim an end-to-end RTP-LLM serving speedup, and it does not route production attention through sparse candidates.

Related follow-up: #1161

Benchmark

Environment:

  • WSL2
  • NVIDIA GeForce RTX 4060 Laptop GPU
  • PyTorch 2.5.1+cu121

Command:

python benchmark/topology_kv_candidate_schedule.py \
  --seq-len 16384 \
  --selected-tokens 128 256 512 1024 \
  --heads 16 \
  --head-dim 64 \
  --rounds 60 \
  --warmup 20 \
  --device cuda

Example output:

| seq_len | selected_tokens | dense_sdpa_ms | sparse_selected_ms | speedup |
| ---: | ---: | ---: | ---: | ---: |
| 16384 | 128 | 0.2816 | 0.0514 | 5.48x |
| 16384 | 256 | 0.2827 | 0.0489 | 5.78x |
| 16384 | 512 | 0.2814 | 0.0752 | 3.74x |
| 16384 | 1024 | 0.2833 | 0.1298 | 2.18x |

Validation

python -m pytest benchmark/test_topology_kv_candidate_schedule.py -q
python -m py_compile benchmark/topology_kv_candidate_schedule.py benchmark/test_topology_kv_candidate_schedule.py
python -m flake8 benchmark/topology_kv_candidate_schedule.py benchmark/test_topology_kv_candidate_schedule.py
python benchmark/topology_kv_candidate_schedule.py --seq-len 16384 --selected-tokens 128 256 512 1024 --heads 16 --head-dim 64 --rounds 60 --warmup 20 --device cuda

Results:

  • 12 passed
  • py_compile passed
  • flake8 passed
  • CUDA benchmark completed successfully

Notes

  • The sparse benchmark path uses the topology schedule, not uniformly spaced token indices.
  • Dense and sparse measurements both use PyTorch scaled_dot_product_attention; only the attended token set changes.
  • CUDA benchmark tests are smoke coverage, not hardware-dependent pass/fail speed gates.
  • Runtime sparse MLA/indexer integration and model-quality validation are intentionally left to follow-up work tracked in Track production validation for topology KV candidate schedules #1161.

@CLAassistant

CLAassistant commented Jul 6, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@teerthsharma teerthsharma marked this pull request as ready for review July 7, 2026 00:03
@teerthsharma teerthsharma requested a review from LLLLKKKK as a code owner July 7, 2026 00:03
Copilot AI review requested due to automatic review settings July 7, 2026 00:03
@LLLLKKKK

LLLLKKKK commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1160

Status: BLOCKING

Summary: P0/0 · P1/0 · P2/3 · P3/1

Non-blocking Suggestions

P2

  • 基准实际未使用其主打的"拓扑候选调度",测得加速与标题主张不符 @ benchmark/topology_kv_candidate_schedule.py:453
    • 建议:让基准真正跑通它宣称的链路:以随机/真实 K 构造 key_block_centroidsbuild_block_candidate_scheduleblock_schedule_to_token_indices 得到候选 token,再喂给稀疏路径计时;或在 README 中明确改写为"均匀等距稀疏基线",避免读者误以为表格反映了拓扑调度的收益。
  • 稠密与稀疏路径为 fused SDPA vs 手写非 fused 实现,比较不对等易高估加速 @ benchmark/topology_kv_candidate_schedule.py:462
    • 建议:保持对比公平:让稀疏路径也走 SDPA(对 gather 后的 K/V 调用 scaled_dot_product_attention),或让稠密路径用相同的手写公式;两侧使用同一 attention 实现,仅变化被 attend 的 token 数,才能干净地隔离出稀疏带来的收益。
  • 提交夹带含作者本地路径的个人 agentic 计划文档,不宜进入主仓库 @ docs/superpowers/plans/2026-07-07-topology-kv-candidate-schedule-validation.md:636
    • 建议:从本 PR 中移除该计划文档(或改为 GitHub issue / PR 描述中的跟踪链接);如需保留设计说明,请去掉本地绝对路径与 agentic 工具指令,改写为面向仓库读者的通用文档。

P3

  • CUDA 基准单测使用硬件相关的性能断言,跨机器易 flaky @ benchmark/test_topology_kv_candidate_schedule.py:206
    • 建议:将该用例定位为"冒烟/可选性能观察"而非硬阈值断言:或仅断言能成功计算出结果、或放宽阈值并允许告警而不失败;如确需 gate 性能,请在固定硬件上运行并说明基线来源。

Checklist Violations (2 fail / 54 total)

General Principles Checklist

  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue 稠密与稀疏路径为 fused SDPA vs 手写非 fused 实现,比较不对等易高估加速
    稠密侧走 F.scaled_dot_product_attention(融合 flash kernel),稀疏侧 _sparse_decode_attention_impl 是手写 matmul + softmax + matmul 的非融合实现。两条路径的 kernel 形态不同,测得的 speedup 把"选中 token 更少"与"kernel 实现差异"混为一谈;理论上非融合稀疏应比融合稠密更慢,若仍更快则加速主要来自 token 数下降,指标含义被稀释。作为对外发布的基准信号,这会误导对稀疏收益量级的判断。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue CUDA 基准单测使用硬件相关的性能断言,跨机器易 flaky
    test_sparse_attention_cuda_benchmark_is_faster_than_dense_sdpa 断言 result.speedup > 1.15sparse_ms < dense_ms。性能阈值断言天然对 GPU 型号、驱动、并发负载敏感,在不同硬件(尤其结合前述 fused vs 非 fused 的不确定性)上可能不稳定。当前该测试未接入任何 BUILD/bazel target,blast radius 很小;但一旦被纳入 CI 就有 flaky 风险。

Strengths

  • 改动完全隔离在 benchmark/,opt-in 且 CUDA 不可用时自动跳过速度门,不影响任何生产 serving 行为。
  • 调度核心逻辑(质心、候选块、token 展开、稀疏 attention 与稠密等价性)有聚焦单测覆盖,包含 padding、2D/3D 输入、全选等价等边界。
  • 输入校验充分(block_size/max_candidate_blocks 正值检查、candidate 越界检查、shape 校验),错误信息清晰。
  • README 对基准定位诚实,明确指出这只是筛选信号、端到端提速仍需运行时集成与模型质量验证。
  • 类型标注规范,输入用 Sequence/Iterable、返回用具体类型;无可变默认参数、无裸 except。

@teerthsharma teerthsharma marked this pull request as draft July 7, 2026 00:16
@teerthsharma teerthsharma marked this pull request as ready for review July 7, 2026 00:19
@teerthsharma teerthsharma marked this pull request as draft July 7, 2026 00:19
@teerthsharma teerthsharma marked this pull request as ready for review July 7, 2026 00:19

Copilot AI 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.

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds an opt-in benchmarking + test harness to evaluate a topology-derived KV candidate schedule for long-context decode attention, comparing dense PyTorch SDPA against a selected-token (gathered K/V) SDPA path.

Changes:

  • Added a topology-style block schedule builder (sink/local/high-drift) plus token-index expansion helpers.
  • Added a CUDA/CPU benchmark script that times dense SDPA vs selected-token SDPA for decode (q_len=1).
  • Added unit tests and benchmark documentation with an example command/output.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
benchmark/topology_kv_candidate_schedule.py Implements centroiding, block candidate scheduling, token expansion, and the dense vs selected-token SDPA benchmark runner.
benchmark/test_topology_kv_candidate_schedule.py Adds correctness + reproducibility tests for centroiding/scheduling and a CUDA-gated benchmark smoke test.
benchmark/README.md Documents how to run the new benchmark and shows example output/caveats.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread benchmark/topology_kv_candidate_schedule.py
Comment thread benchmark/topology_kv_candidate_schedule.py Outdated
Comment thread benchmark/topology_kv_candidate_schedule.py Outdated
Comment thread benchmark/topology_kv_candidate_schedule.py Outdated
Comment thread benchmark/README.md
Comment thread benchmark/README.md Outdated
Copilot AI review requested due to automatic review settings July 7, 2026 00:20
@teerthsharma teerthsharma marked this pull request as draft July 7, 2026 00:21
@teerthsharma teerthsharma marked this pull request as ready for review July 7, 2026 00:26

Copilot AI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread benchmark/topology_kv_candidate_schedule.py Outdated
Comment thread benchmark/topology_kv_candidate_schedule.py
Comment thread benchmark/README.md Outdated
Copilot AI review requested due to automatic review settings July 7, 2026 00:37
@LLLLKKKK

LLLLKKKK commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1160

Status: BLOCKING

Summary: P0/0 · P1/0 · P2/0 · P3/2

Non-blocking Suggestions

P3

  • selected_tokens 非 block_size 整数倍时截断丢弃最新 local token @ benchmark/topology_kv_candidate_schedule.py:200
    • 建议:若希望预算不对齐时优先保留最新 local token,可在截断前按「local 优先」重排,或在取前 N 时对末尾 block 做保留处理;至少在 docstring 中说明「非 block 对齐预算下优先保留 sink 而非最新 token」的实际语义,避免误用。
  • 仅使用最后一行 schedule 却构造全量 schedule @ benchmark/topology_kv_candidate_schedule.py:188
    • 建议:可为「只需最后一个 query block 候选」提供轻量路径,直接对 query_block = block_count - 1 计算单行 schedule,避免构造并丢弃全量矩阵;或在注释中说明当前为复用现成 API 的有意取舍。

Checklist Violations (2 fail / 54 total)

General Principles Checklist

  • [6.1] Software Engineering — KISS/YAGNI:无投机性抽象 → issue 仅使用最后一行 schedule 却构造全量 schedule
    build_topology_candidate_token_indices 调用 build_block_candidate_schedule 生成所有 query_block 的候选行(O(block_count²) 的 Python 循环 + 每 block 的 append/sort),但随后仅用 schedule[-1:] 最后一行,其余全部丢弃。对长序列(如 seq_len=16384, block_size=64 → 256 个 block)会产生不必要的构造开销。此为 benchmark 代码、非服务热路径,影响有限。
  • [6.1] Tests — 边界 case 覆盖(空、单元素、最大值) → issue selected_tokens 非 block_size 整数倍时截断丢弃最新 local token
    build_topology_candidate_token_indices 中 schedule row 经 sorted(row) 升序排列,block_schedule_to_token_indices 也按升序展开,最终 return token_indices[:selected_tokens] 从头截断。当 selected_tokens 不是 block_size 整数倍时(例如 selected_tokens=100, block_size=64,max_candidate_blocks=2 → 展开 128 token),截断保留最靠前的 sink block、丢弃末尾最新的 local block token,这与文档声明「local blocks keep the newest causal context」相悖。默认网格 128/256/512/1024 均为 64 的整数倍,恰好规避,故仅为潜在行为。

Strengths

  • 变更彻底隔离在 benchmark/,不改动 serving/推理行为,风险面小,回滚代价为零。
  • 核心逻辑(centroid 聚合、schedule 构造、block→token 展开、dense/sparse 一致性)均由纯函数实现,且单测覆盖了 sink/local/salience 组合、短行 padding、2D/3D/4D 输入、dense=sparse 全选一致性、重复索引拒绝等边界。
  • 输入校验充分:block_size/seq_len/selected_tokens 非法值 fail-fast,sparse_decode_attention 显式拒绝重复 token 与越界索引,错误语义清晰。
  • README 对该 benchmark 的定位与局限做了诚实说明,未夸大端到端收益。

@LLLLKKKK

LLLLKKKK commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1160

Status: BLOCKING

Summary: P0/0 · P1/0 · P2/1 · P3/1

Non-blocking Suggestions

P2

  • 新增单测未接入任何自动化执行入口 @ benchmark/test_topology_kv_candidate_schedule.py:1
    • 建议:至少在 README 的基准章节补一行说明测试的正确运行方式(如"从仓库根执行 python -m unittest benchmark.test_topology_kv_candidate_schedule");若希望长期防回归,建议为其补一个 py_test BUILD 目标或纳入现有 Python 测试发现范围,使 CI 能自动执行 CPU 可跑的用例(CUDA 用例已用 skipUnless 正确 gate)。

P3

  • speedup 计算未防护 sparse_ms 为 0 的除零风险 @ benchmark/topology_kv_candidate_schedule.py:623
    • 建议:在计算 speedup 前对 sparse_ms 做一次 > 0 判定(为 0 时记为 float("inf") 或跳过并 warn),使基准在极端计时分辨率下仍能给出稳健输出。

Checklist Violations (1 fail / 54 total)

General Principles Checklist

  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue 新增单测未接入任何自动化执行入口
    单测文件通过 from benchmark.topology_kv_candidate_schedule import ... 绝对包路径导入,但 benchmark/ 下无 __init__.py、也无对应 BUILD py_test 目标,仓库 CI(bazel test)不会覆盖 benchmark 目录。这意味着这些测试只能在仓库根手动 python -m unittest 运行;若在 benchmark/ 子目录内直接运行则该导入会失败,且后续对脚本的改动不会被 CI 回归拦截,测试的保护价值大打折扣。

Strengths

  • 输入校验充分:build_key_block_centroids / sparse_decode_attention / build_topology_candidate_token_indices 对张量维度、block_size/selected_tokens 正数、key/value 形状一致、候选 index 去重与越界均做了显式 fail-fast 校验并抛出可读的 ValueError
  • 单测覆盖到位:既覆盖了调度语义(sink/local/salience、短序列 padding、partial budget 保留最新 token),又用 test_sparse_attention_matches_dense_when_all_tokens_are_selected 建立了稀疏路径与稠密路径的等价性回归,作为基准正确性锚点。
  • 变更边界清晰:仅新增 benchmark 独立脚本,README 明确标注为 benchmark 信号、非 serving 行为变更,且不引入对在线代码的任何依赖,符合 KISS/YAGNI 与分层边界原则。
  • dataclass(frozen=True) 承载配置与结果、docstring 说明语义,代码可读性好。

Copilot AI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

Comment thread benchmark/topology_kv_candidate_schedule.py
Comment thread benchmark/topology_kv_candidate_schedule.py
Comment thread benchmark/topology_kv_candidate_schedule.py
Comment thread benchmark/topology_kv_candidate_schedule.py
Comment thread benchmark/topology_kv_candidate_schedule.py Outdated
Comment thread benchmark/README.md
Copilot AI review requested due to automatic review settings July 7, 2026 00:54
@LLLLKKKK

LLLLKKKK commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1160

Status: LGTM

Summary: P0/0 · P1/0 · P2/2 · P3/1

lgtm ready to ci

Non-blocking Suggestions

P2

  • 候选选择开销被排除在计时之外,可能夸大稀疏加速比 @ benchmark/topology_kv_candidate_schedule.py:604
    • 建议:README 已注明「仅作为 benchmark 信号」,建议进一步在文档中显式说明「稀疏路径的耗时不含候选选择/schedule 构建成本,仅含 gather + SDPA」,或提供一个把选择开销计入的对照列,避免读者把该 speedup 误读为可直接落地的加速。
  • 候选 token 数在非整块 seq_len 下可能静默少于请求值 @ benchmark/topology_kv_candidate_schedule.py:549
    • 建议:建议在返回前校验实际 token 数是否达到 selected_tokens:不足时要么显式抛错(与其余入参校验风格一致),要么用返回张量的真实长度作为结果表的 selected_tokens,使表格列名与实际选中 token 数严格对应,避免未来集成时误判。

P3

  • 测试文件的直接执行入口 __main__ 实际不可用 @ benchmark/test_topology_kv_candidate_schedule.py:278
    • 建议:要么去掉误导性的 __main__ 入口,要么在 README 里明确注明测试必须从仓库根以 python -m unittest ... / pytest 方式运行;如希望支持直接执行,可在测试顶部插入仓库根到 sys.path,或为 benchmark/ 增加 __init__.py 并统一导入方式,避免使用者踩坑。

Checklist ✅ (54 items passed)

Strengths

  • 纯新增、opt-in 基准工具,不改变任何 serving 行为,且 README 明确标注结果只是信号、不代表端到端加速,边界表达诚实。
  • 核心函数(build_key_block_centroids / block_schedule_to_token_indices / build_topology_candidate_token_indices / sparse_decode_attention)均有输入校验(block_size/seq_len 正数、shape 校验、候选去重、越界检查),并对 2D/3D/4D 输入做了归一化处理。
  • 单测覆盖了 schedule 生成、padding、稀疏与 dense 等价性、去重拒绝、block→token 展开、部分预算下的最新 block 优先、以及 grid/markdown 可复现性,边界 case(单 block 预算、非整块预算)都有专门断言。
  • dense_decode_attentionis_causal=False 的选择有准确注释说明(q_len=1 decode 不应误用 causal mask),避免了常见误用。
  • 计时把 index_select 收集开销计入稀疏循环、并在 CUDA 上正确使用 torch.cuda.synchronize,计时方式基本合理。

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comment thread benchmark/topology_kv_candidate_schedule.py
Comment thread benchmark/topology_kv_candidate_schedule.py Outdated
Comment thread benchmark/topology_kv_candidate_schedule.py
@teerthsharma teerthsharma marked this pull request as draft July 9, 2026 10:55
@teerthsharma teerthsharma marked this pull request as ready for review July 9, 2026 11:00
Copilot AI review requested due to automatic review settings July 9, 2026 11:02

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@LLLLKKKK

LLLLKKKK commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1160

Status: LGTM

Summary: P0/0 · P1/0 · P2/1 · P3/0

lgtm ready to ci

Non-blocking Suggestions

P2

  • partial final block 场景会跳过最近上下文 token @ benchmark/topology_kv_candidate_schedule.py:310
    • 建议:当 final block 是 partial 且预算不足时,先保证最新 causal token/block 的预算,再分配 sink/drift;或者在 token 级裁剪前把上一块尾部 token 排在 sink token 之前。补充覆盖 seq_len=65, selected_tokens=64, block_size=64seq_len=129, selected_tokens=64, block_size=64 这类边界用例,断言最近上下文不会被 sink 预算挤掉。

Checklist ✅ (44 items passed)

Strengths

  • 新增 benchmark 明确标注为 opt-in,README 也说明 speedup 不等同于端到端模型收益,避免把筛选实验误认为生产加速。
  • 测试覆盖了 sink/local/drift 组合、padding、空序列、dtype 累加、重复 candidate 拒绝、device fallback 和 RNG 不污染等关键辅助行为。

@wht21

wht21 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

Copilot AI review requested due to automatic review settings July 11, 2026 04:38

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1160

Status: BLOCKING

Summary: P0/0 · P1/1 · P2/0 · P3/0

Blocking Issues

P1

  • 新增回归测试未接入 CI @ benchmark/README.md:55
    • 建议:为 CPU 回归增加非 manualpy_test 并纳入实际 CI 测试集合,或将其迁移到已有 CI 覆盖的测试 package;耗时的 CUDA 性能用例可以继续单独标记为手工或 GPU 条件测试。

Checklist Violations (2 fail / 50 total)

General Principles Checklist

  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue 新增回归测试未接入 CI
    benchmark/test_topology_kv_candidate_schedule.py 新增了完整回归用例,但当前 benchmark/ 下没有 BUILD,全仓也不存在引用该测试的 Bazel/CI target;README 仅提供手工 python -m unittest 命令。因此常规 CI 不会执行这些断言,候选预算、尾块顺序或计时路径后续退化时不会产生检查信号。

RTP-LLM Checklist

  • [H] 测试与 CI — 新增、迁移或删除测试必须证明目标行为仍在 CI 中执行;DISABLED_、#if 0、open_skip、导入即失败、未被 target 引用的用例不得计为覆盖 → issue 新增回归测试未接入 CI
    benchmark/test_topology_kv_candidate_schedule.py 新增了完整回归用例,但当前 benchmark/ 下没有 BUILD,全仓也不存在引用该测试的 Bazel/CI target;README 仅提供手工 python -m unittest 命令。因此常规 CI 不会执行这些断言,候选预算、尾块顺序或计时路径后续退化时不会产生检查信号。

Strengths

  • 候选调度显式保留 query block,并覆盖 sink、local、drift、尾块不足和部分预算等边界。
  • README 清楚限定该结果仅为 benchmark 信号,未将其表述为运行时收益或模型质量结论。

Copilot AI review requested due to automatic review settings July 11, 2026 04:57

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1160

Status: BLOCKING

Summary: P0/0 · P1/1 · P2/0 · P3/0

Blocking Issues

P1

  • Bazel 测试依赖了 C++ Torch 目标,无法可靠导入 Python 包 @ benchmark/BUILD:10
    • 建议:将两个目标的依赖改为平台感知的 Python 目标 //rtp_llm:torch,移除不再需要的 torch_deps load,并在不依赖宿主 site-packages 的 Bazel 环境中验证 CPU 与显式 CUDA 目标均能完成导入和执行。

Checklist Violations (3 fail / 63 total)

General Principles Checklist

  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue Bazel 测试依赖了 C++ Torch 目标,无法可靠导入 Python 包
    两个 py_test 都执行 import torch,但 torch_deps() 选择的是 BUILD.pytorch 中仅提供头文件和 .socc_library 目标,不会声明 rules_python 的 Torch 包。仓库内 Python 测试使用 //rtp_llm:torch。因此干净 Bazel runfiles 中会导入失败,或仅因宿主环境预装 Torch 而偶然通过,使新增 CPU CI 覆盖不可靠。

RTP-LLM Checklist

  • [H] 测试与 CI — BUILD、py_test、cc_test、genrule 变更必须验证 srcs/data/runfiles/testdata 相对路径、import path 和 target 可执行性;genrule glob 不得捕获无关构建产物 → issue Bazel 测试依赖了 C++ Torch 目标,无法可靠导入 Python 包
    两个 py_test 都执行 import torch,但 torch_deps() 选择的是 BUILD.pytorch 中仅提供头文件和 .socc_library 目标,不会声明 rules_python 的 Torch 包。仓库内 Python 测试使用 //rtp_llm:torch。因此干净 Bazel runfiles 中会导入失败,或仅因宿主环境预装 Torch 而偶然通过,使新增 CPU CI 覆盖不可靠。
  • [H] 测试与 CI — 新增、迁移或删除测试必须证明目标行为仍在 CI 中执行;DISABLED_、#if 0、open_skip、导入即失败、未被 target 引用的用例不得计为覆盖 → issue Bazel 测试依赖了 C++ Torch 目标,无法可靠导入 Python 包
    两个 py_test 都执行 import torch,但 torch_deps() 选择的是 BUILD.pytorch 中仅提供头文件和 .socc_library 目标,不会声明 rules_python 的 Torch 包。仓库内 Python 测试使用 //rtp_llm:torch。因此干净 Bazel runfiles 中会导入失败,或仅因宿主环境预装 Torch 而偶然通过,使新增 CPU CI 覆盖不可靠。

Strengths

  • README 明确说明调度构建不计入稀疏路径计时,且 benchmark 结果不能直接代表端到端收益。
  • 单测覆盖 query block 保留、尾块、部分 token 预算、重复索引、全量候选等关键边界。
  • benchmark 使用局部随机数生成器,并在 CUDA 计时边界执行同步,避免污染全局 RNG 或漏计异步执行时间。

Copilot AI review requested due to automatic review settings July 11, 2026 12:53

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1160

Status: LGTM

Summary: P0/0 · P1/0 · P2/0 · P3/0

lgtm ready to ci

Checklist ✅ (63 items passed)

Strengths

  • Bazel 测试现在使用仓库统一的 Python Torch 依赖,并保留 CPU 自动执行与 CUDA 手动性能测试的隔离。
  • 单测覆盖 query block 保留、部分 token 预算、尾块、重复索引、全量候选和设备 fallback 等关键边界。
  • README 明确区分候选调度构建与计时路径,并避免将微基准结果表述为端到端 serving 收益。

@wht21

wht21 commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

Copilot AI review requested due to automatic review settings July 11, 2026 13:29

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1160

Status: BLOCKING

Summary: P0/0 · P1/1 · P2/0 · P3/0

Blocking Issues

P1

  • ROCm 配置跳过全部自动化回归 @ benchmark/BUILD:9
    • 建议:移除该 ROCm 排除条件,让确定性 CPU 回归在 ROCm 配置下继续执行。若平台依赖确有问题,应修复对应 Torch target 或仅隔离真正不支持的 GPU 性能用例,并补充 ROCm 配置下的执行证据。

Checklist Violations (2 fail / 70 total)

General Principles Checklist

  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue ROCm 配置跳过全部自动化回归
    --config=rocm 会将唯一非 manual 的 CPU 回归目标标记为 incompatible,而另一个目标仍带有 manual 标签,因此 ROCm CI 不会执行本 PR 的任何调度或正确性测试。该 CPU 目标固定使用 CPU tensor/SDPA,代码中也未说明必须排除 ROCm 的不兼容点;当前修改实际以跳过测试代替了平台问题的定位或修复。

RTP-LLM Checklist

  • [H] 测试与 CI — 新增、迁移或删除测试必须证明目标行为仍在 CI 中执行;DISABLED_、#if 0、open_skip、导入即失败、未被 target 引用的用例不得计为覆盖 → issue ROCm 配置跳过全部自动化回归
    --config=rocm 会将唯一非 manual 的 CPU 回归目标标记为 incompatible,而另一个目标仍带有 manual 标签,因此 ROCm CI 不会执行本 PR 的任何调度或正确性测试。该 CPU 目标固定使用 CPU tensor/SDPA,代码中也未说明必须排除 ROCm 的不兼容点;当前修改实际以跳过测试代替了平台问题的定位或修复。

Strengths

  • benchmark 与生产 serving 路径保持隔离,变更不会直接影响在线推理行为。
  • 单测覆盖候选预算、尾块、query block、重复索引、全量候选等关键边界。
  • README 明确区分候选构建与计时范围,没有将微基准结果表述为端到端收益。

Copilot AI review requested due to automatic review settings July 11, 2026 13:41

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1160

Status: LGTM

Summary: P0/0 · P1/0 · P2/0 · P3/0

lgtm ready to ci

Checklist ✅ (70 items passed)

Strengths

  • 调度构建明确排除在计时区间之外,稀疏计时包含 K/V gather 与 SDPA,README 对结果适用范围和质量验证要求描述准确。
  • CPU 回归覆盖 sink/local/drift 选择、尾块与部分预算、dense/sparse 等价性、设备选择及 RNG 隔离;CUDA 性能用例通过 manual 标签与常规 CI 分离。
  • CPU target 使用平台选择的 Python torch 依赖,ROCm 解禁后执行路径仍限定为通用 CPU tensor 操作。

@wht21

wht21 commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

@teerthsharma

Copy link
Copy Markdown
Author

以下为英文说明的中文翻译,供审核参考:

您好,最新 CI 仍失败于内部 build-amd job(78794077)。带 ROCm 排除条件和移除该条件的两个版本都在同一阶段失败,因此目前无法确认 ROCm guard 是根因。GitHub wrapper 未提供实际编译/测试错误,烦请协助提供内部 job 日志,以便进行针对性修复。谢谢。 @LLLLKKKK

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