Skip to content

knowledge(vllm-omni): 2 change(s) from refine - #343

Merged
tzhouam merged 1 commit into
mainfrom
kb/vllm-omni/vllm-omni-refine-25fee39dda30
Oct 10, 2026
Merged

tzhouam merged 1 commit into
mainfrom
kb/vllm-omni/vllm-omni-refine-25fee39dda30

Conversation

@tzhouam

@tzhouam tzhouam commented Oct 10, 2026

Copy link
Copy Markdown
Collaborator

Generated automatically by the Copilot knowledge service (changeset vllm-omni-refine-25fee39dda30).

op page rule
add repos/vllm-omni/components/model-executor/rules-numeric-exactness.md VLLM-OMNI-PR8562-TRITON-HIP-RN
add repos/vllm-omni/components/model-executor/rules-numeric-exactness.md VLLM-OMNI-PR8562-GRAPH-BUCKET-ORACLE

Generator: zcode:GLM-5.3 · judge: codex:gpt-6-sol:medium.
The publisher merges it only if its local gate passes on the exact merge result.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-10T04:05:13.369860Z 801c3a1 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 801c3a1df5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +10 to +12
# 数值精确性与平台可移植性规则

## VLLM-OMNI-PR8562-TRITON-HIP-RN — Triton libdevice 显式舍入算子必须按平台编译期分支

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Split this page by the actual model owners

This page combines a MOSS-specific _rotate/HIP contract with a Breeze-specific graph-oracle contract under a generic numeric-exactness topic because both came from PR #8562. The existing MOSS owner is knowledge/repos/vllm-omni/models/moss-tts/, and the existing Breeze platform topic is rules-breeze-rocm.md; owner-routed reviews can therefore miss these rules or load only part of the applicable contract. Move or merge each conclusion into its nearest existing owner rule rather than creating this cross-model page.

AGENTS.md reference: AGENTS.md:L12-L17

Useful? React with 👍 / 👎.

- 触发:在 `vllm_omni/model_executor/models/` 下的 Triton kernel(如 `moss_tts/first_frame_special.py` 的 `_rotate`)调用 CUDA libdevice 内建(`add_rn`/`mul_rn` 等显式舍入算子),或修改这类 kernel 的跨平台数值路径(本例的修复对象是 AMD READY R2-01 暴露的 MOSS first-frame attention 失败)。
- 强制:CUDA-only 的 libdevice 算子必须在 Triton 编译期用单一 `tl.constexpr` 按平台选择(PR 的形态:模块级 `_CUDA_RN_OPS = current_omni_platform.is_cuda()` 作为 constexpr 默认参数):NVIDIA CUDA 保留显式 round-to-nearest 路径,非 CUDA 平台(含 HIP/ROCm)改走可移植的 FP32 乘加表达式(`value_fp32 * cosine + signed_partner * sine`);与原始实现(reference attention)的既有数值对照不得因平台分支而放松。
- 禁止:让 HIP 平台编译进 Triton HIP mapping 中不存在的 libdevice 调用(PR 记录 `add_rn` 与配对的显式 RN 算子在 HIP mapping 均无映射);把平台判断做成 kernel 内运行时分支或多个互相不一致的常量。
- 验收:审查者核对平台选择是单一编译期常量、CUDA 与可移植两种表达式在 kernel 内显式分支、既有数值对照未放松,并要求申请者分别给出两条平台路径的数值测试和修复后目标平台的 CI 通过证据。PR #8562 自身的证据不满足后两项:所示 diff 除 `_rotate` kernel 外还改了两个测试文件——`test_audio_embed_kernel.py` 把随机 reduction 断言放宽为默认 `assert_close` 并新增可精确表示的 bitwise 用例、`test_breeze_tts_2_graphs.py` 重写 Breeze graph oracle——但所示改动都在 CUDA 上运行,不触及 `_rotate` 的任何平台分支;PR body 明确保持 R2-01 `NonBlocking` 并把绿色 soak 推迟到后续 promote PR,证据未附修复后的 AMD CI 通过记录。^[PR #8562]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Remove transient PR results from the executable rule

The acceptance clause persists PR #8562's changed-test inventory, body status, and missing AMD CI result. Those are transient review/result history rather than a reusable trigger, requirement, prohibition, or acceptance condition, and they will become stale if the deferred promotion later supplies the missing evidence. Keep the platform-test and target-CI requirements, retain the PR only as provenance, and drop the replay-style account of what that PR did not prove.

AGENTS.md reference: AGENTS.md:L12-L17

Useful? React with 👍 / 👎.

sources: ["PR #8562"]
---

# 数值精确性与平台可移植性规则

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add the required related-page links

The new rule page contains no relative Markdown links, while doc/knowledge/SCHEMA.md:94-96 requires every sedimented knowledge page to link at least two related pages. The ^[PR #8562] provenance markers are not links, so readers cannot navigate from this page to the existing MOSS, Breeze, or platform contracts; add at least two relevant owner-page links.

AGENTS.md reference: AGENTS.md:L7-L10

Useful? React with 👍 / 👎.

@tzhouam

tzhouam commented Oct 10, 2026

Copy link
Copy Markdown
Collaborator Author

Knowledge quality gate · checked 2026-10-10 at 801c3a1df5b5 (checked again once a day)

Result: not passed. This PR will not be merged until the problems below are fixed; push a fix and it is checked again at the next daily pass.

Problems:

  • L2 was not sure about VLLM-OMNI-PR8562-TRITON-HIP-RN, VLLM-OMNI-PR8562-GRAPH-BUCKET-ORACLE

@tzhouam
tzhouam merged commit 09e139c into main Oct 10, 2026
3 checks passed
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.

1 participant