Repository navigation
Conversation
AI Review审查尚未完整完成，无法给出整体结论；当前没有可发布的已确认问题，不代表代码没有问题。
|
xgbah
force-pushed
the
feat/openvla-optimization
branch
from
October 9, 2026 05:44
3715611 to
ca02803
Compare
![hygon-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/280652917?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review
🟡 建议修改
发现有明确证据的问题，建议核对并处理。
已确认 1 个问题，其中 1 个已添加到对应代码行。
变更概览
本次变更主要包含：
- 新增 OpenVLA 应用文档，说明如何基于官方 OpenVLA 仓库（指定 commit）准备数据与权重，并通过 turbo-physai run 以优化配置启动单机八卡训练；同时在模型支持清单中登记 OpenVLA-7B 及其基线 commit。
- 本批新增 test_openvla_skip_fa2_unpad.py，覆盖 openvla.llm.skip_fa2_unpad 组：断言包装器在 FA2 右 padding prefill 时返回 None 且不调用原函数，eager/sdpa/decode/无掩码路径原样委托，组声明经引擎 Preparation 解析，并提供 CUDA bf16 FA2 跳过 unpad 路径的数值一致性测试
- 新增跳过 FA2 unpad 优化路径的集成测试：对比默认与替换后 _update_causal_mask 的输出、loss 与逐参数梯度是否逐位一致，并用断言确保两条路径分别真实进入/跳过 unpad，同时验证全真实 token 批次下默认路径本就不触发 unpad。
- 新增 turbo_physai.optimizations.models.openvla 包的 __init__.py，导入并导出 catalog 模块作为该模型优化目录的包入口。
文件审查摘要
| 文件 | 变更 | 审查结果 |
|---|---|---|
turbo_physai/optimizations/models/openvla/skip_fa2_unpad.py |
新增 · +39/-0 | P2 × 1 |
turbo_physai/optimizations/models/openvla/reproducibility.py |
新增 · +673/-0 | — |
turbo_physai/optimizations/models/openvla/compile_fsdp1.py |
新增 · +377/-0 | — |
test/optimizations/test_openvla_skip_fa2_unpad.py |
新增 · +321/-0 | — |
turbo_physai/optimizations/models/openvla/spawn_dataloader.py |
新增 · +270/-0 | — |
test/optimizations/test_openvla_vision_timm.py |
新增 · +205/-0 | — |
turbo_physai/optimizations/models/openvla/catalog.py |
新增 · +200/-0 | — |
turbo_physai/optimizations/models/openvla/configs/optimization.yaml |
新增 · +136/-0 | — |
turbo_physai/optimizations/models/openvla/text_len_bucket.py |
新增 · +107/-0 | — |
model_examples/OpenVLA/README.md |
新增 · +99/-0 | — |
turbo_physai/optimizations/models/openvla/vision_timm.py |
新增 · +98/-0 | — |
turbo_physai/optimizations/models/openvla/fsdp_prefetch.py |
新增 · +88/-0 | — |
turbo_physai/optimizations/models/openvla/configs/recipe.yaml |
新增 · +83/-0 | — |
turbo_physai/optimizations/models/openvla/gc_freeze.py |
新增 · +51/-0 | — |
turbo_physai/optimizations/models/openvla/fixbf16support.py |
新增 · +36/-0 | — |
turbo_physai/optimizations/models/openvla/configs/runtime.yaml |
新增 · +25/-0 | — |
turbo_physai/optimizations/models/openvla/configs/.optimization.yaml.generation.json |
新增 · +23/-0 | — |
turbo_physai/optimizations/models/openvla/fusedAdamW.py |
新增 · +15/-0 | — |
turbo_physai/optimizations/models/openvla/init.py |
新增 · +8/-0 | — |
turbo_physai/optimizations/models/openvla/configs/init.py |
新增 · +4/-0 | — |
其余 1 个文件未逐项展开。
审查信息
- 变更统计：21 个文件，+2859/-0。
- 覆盖情况：共 21 个文件，已完整审查 21 个。
- 候选问题：1 项；证据复核过滤：0 项；发布前敏感信息保护：0 项。
- 本服务以 GitHub 提供的 PR diff 为审查主体，PR 描述与按相关性选取的仓库片段仅用于核验；未执行代码或重跑测试，结论仍需维护者核验。
| # Prefill (no KV cache) + FA2 + an explicit mask: skip the causal-mask | ||
| # materialisation entirely so FA2 never enters the varlen/unpad path. | ||
| if ( | ||
| self.config._attn_implementation == "flash_attention_2" |
There was a problem hiding this comment.
P2 · 跳过 FA2 unpad 未按声明检测左侧 padding，左 padding 预填会静默产生错误注意力
触发条件： Group 启用时，同一被包装模型收到一批左侧 padding 输入（attention_mask 非空、past_seen_tokens==0、FA2），例如该预填路径被复用于批量生成等场景。
证据与影响： wrapper 仅检查 _attn_implementation、attention_mask 非空和 past_seen_tokens==0 即返回 None；未实现 PR 描述声称的“左侧 padding 一律回落原逻辑”，缺少 padding 侧检测。左 padding 时真实 token 经 causal 注意力会 attend 到左侧 pad 位置的 key/value。 与上游 varlen 路径数值不一致：真实 token 输出被 pad 表示污染且不报错，属于相对上游行为的回归，与描述的安全前提矛盾。
修改建议： 在返回 None 前增加 padding 侧检查（如根据 attention_mask 首列是否全为有效判定右 padding），不满足右 padding 条件时回落 original，保持与声明的回落条件一致。
Add `model_examples/OpenVLA/README.md`, required by CONTRIBUTING for a newly supported model: model overview, the upstream OpenVLA commit the optimizations were validated against (`c8f03f4`), source / BridgeData V2 / base-VLM preparation, and the `turbo-physai run` command for single-node 8-accelerator full fine-tuning.
Record the OpenVLA optimization baseline (`c8f03f4`) and link the new application guide from `docs/zh/models/support_list.md`.
xgbah
force-pushed
the
feat/openvla-optimization
branch
from
October 9, 2026 06:26
ca02803 to
e7aaad0
Compare
There was a problem hiding this comment.
AI Review
🟢 未发现有明确证据的问题
在已提供并完成审查的 diff 范围内未形成可确认问题。
变更概览
本次变更主要包含：
- 新增 OpenVLA 模型支持文档：在支持清单中登记 OpenVLA-7B 及其接入基线 commit，并新增应用说明文档，描述数据集/权重准备与通过 turbo-physai run 启动训练的步骤。
- 新增 openvla.llm.skip_fa2_unpad 优化组的测试文件：用替身模型验证包装器在 FA2 右侧 padding prefill 时返回 None 且不调用原函数，其余路径（eager/sdpa、缓存解码、无 mask、左 padding、无 padding）均回落原逻辑，并通过引擎 Preparation 入口校验声明解析。
- 本批新增 openvla.llm.skip_fa2_unpad 优化的测试：用 spy 计数确认默认路径进入 FA2 varlen unpad 而启用包装器后跳过该路径，并断言真实 token 的输出、loss 与各参数梯度逐位一致，全真实 batch 下两种路径均为无操作。
- 新增 OpenVLA 优化目录包入口 turbo_physai/optimizations/models/openvla/__init__.py，导入并暴露 catalog 模块。
文件审查摘要
| 文件 | 变更 | 审查结果 |
|---|---|---|
turbo_physai/optimizations/models/openvla/reproducibility.py |
新增 · +673/-0 | — |
turbo_physai/optimizations/models/openvla/compile_fsdp1.py |
新增 · +377/-0 | — |
test/optimizations/test_openvla_skip_fa2_unpad.py |
新增 · +368/-0 | — |
turbo_physai/optimizations/models/openvla/spawn_dataloader.py |
新增 · +270/-0 | — |
test/optimizations/test_openvla_vision_timm.py |
新增 · +205/-0 | — |
turbo_physai/optimizations/models/openvla/catalog.py |
新增 · +200/-0 | — |
turbo_physai/optimizations/models/openvla/configs/optimization.yaml |
新增 · +136/-0 | — |
turbo_physai/optimizations/models/openvla/text_len_bucket.py |
新增 · +107/-0 | — |
model_examples/OpenVLA/README.md |
新增 · +99/-0 | — |
turbo_physai/optimizations/models/openvla/vision_timm.py |
新增 · +98/-0 | — |
turbo_physai/optimizations/models/openvla/fsdp_prefetch.py |
新增 · +88/-0 | — |
turbo_physai/optimizations/models/openvla/configs/recipe.yaml |
新增 · +83/-0 | — |
turbo_physai/optimizations/models/openvla/skip_fa2_unpad.py |
新增 · +59/-0 | — |
turbo_physai/optimizations/models/openvla/gc_freeze.py |
新增 · +51/-0 | — |
turbo_physai/optimizations/models/openvla/fixbf16support.py |
新增 · +36/-0 | — |
turbo_physai/optimizations/models/openvla/configs/runtime.yaml |
新增 · +25/-0 | — |
turbo_physai/optimizations/models/openvla/configs/.optimization.yaml.generation.json |
新增 · +23/-0 | — |
turbo_physai/optimizations/models/openvla/fusedAdamW.py |
新增 · +15/-0 | — |
turbo_physai/optimizations/models/openvla/init.py |
新增 · +8/-0 | — |
turbo_physai/optimizations/models/openvla/configs/init.py |
新增 · +4/-0 | — |
其余 1 个文件未逐项展开。
审查信息
- 变更统计：21 个文件，+2926/-0。
- 覆盖情况：共 21 个文件，已完整审查 21 个。
- 候选问题：0 项；证据复核过滤：0 项；发布前敏感信息保护：0 项。
- 本服务以 GitHub 提供的 PR diff 为审查主体，PR 描述与按相关性选取的仓库片段仅用于核验；未执行代码或重跑测试，结论仍需维护者核验。
There was a problem hiding this comment.
AI Review
🔴 需要修改
发现有明确证据的高优先级问题。
已确认 1 个问题，其中 1 个已添加到对应代码行。
变更概览
本次变更主要包含：
- 新增 OpenVLA 模型支持：在支持清单登记 OpenVLA-7B（DINOv2+SigLIP，基线 commit c8f03f4），并新增应用文档，说明模型源码准备、BridgeData V2 数据下载、基座 VLM 权重准备及通过 turbo-physai run 启动单机八卡 FSDP 训练的命令
- 新增 openvla.llm.skip_fa2_unpad 组的单元测试，验证 FA2 右 padding prefill 时包装器返回 None 以跳过 unpad/varlen 路径，左 padding、decode、非 FA2 及无 mask 时回落原逻辑，并通过 Preparation 对真实 Llama 目标做配置解析校验。
- 新增 OpenVLA 右 padding prefill 跳过 FA2 varlen(unpad) 路径的单元测试，比较包装 _update_causal_mask 前后输出、损失与梯度位级一致，并通过调用计数验证两个代码路径均被实际触达。
- 新增针对 OpenVLA FSDP1 视觉塔 timm 重写的单元测试：验证 dynamo_safe_intermediate_layers 在整型、集合、列表、元组、range 等多种 n 形式及各 keyword 参数下与 timm 原实现逐位一致，并验证工厂仅在 compile 开启时才替换方法。
文件审查摘要
| 文件 | 变更 | 审查结果 |
|---|---|---|
turbo_physai/optimizations/models/openvla/skip_fa2_unpad.py |
新增 · +59/-0 | P1 × 1 |
turbo_physai/optimizations/models/openvla/reproducibility.py |
新增 · +673/-0 | — |
turbo_physai/optimizations/models/openvla/compile_fsdp1.py |
新增 · +377/-0 | — |
test/optimizations/test_openvla_skip_fa2_unpad.py |
新增 · +368/-0 | — |
turbo_physai/optimizations/models/openvla/spawn_dataloader.py |
新增 · +270/-0 | — |
test/optimizations/test_openvla_vision_timm.py |
新增 · +205/-0 | — |
turbo_physai/optimizations/models/openvla/catalog.py |
新增 · +200/-0 | — |
turbo_physai/optimizations/models/openvla/configs/optimization.yaml |
新增 · +136/-0 | — |
turbo_physai/optimizations/models/openvla/text_len_bucket.py |
新增 · +107/-0 | — |
model_examples/OpenVLA/README.md |
新增 · +99/-0 | — |
turbo_physai/optimizations/models/openvla/vision_timm.py |
新增 · +98/-0 | — |
turbo_physai/optimizations/models/openvla/fsdp_prefetch.py |
新增 · +88/-0 | — |
turbo_physai/optimizations/models/openvla/configs/recipe.yaml |
新增 · +83/-0 | — |
turbo_physai/optimizations/models/openvla/gc_freeze.py |
新增 · +51/-0 | — |
turbo_physai/optimizations/models/openvla/fixbf16support.py |
新增 · +36/-0 | — |
turbo_physai/optimizations/models/openvla/configs/runtime.yaml |
新增 · +25/-0 | — |
turbo_physai/optimizations/models/openvla/configs/.optimization.yaml.generation.json |
新增 · +23/-0 | — |
turbo_physai/optimizations/models/openvla/fusedAdamW.py |
新增 · +15/-0 | — |
turbo_physai/optimizations/models/openvla/init.py |
新增 · +8/-0 | — |
turbo_physai/optimizations/models/openvla/configs/init.py |
新增 · +4/-0 | — |
其余 1 个文件未逐项展开。
审查信息
- 变更统计：21 个文件，+2926/-0。
- 覆盖情况：共 21 个文件，已完整审查 21 个。
- 候选问题：1 项；证据复核过滤：0 项；发布前敏感信息保护：0 项。
- 本服务以 GitHub 提供的 PR diff 为审查主体，PR 描述与按相关性选取的仓库片段仅用于核验；未执行代码或重跑测试，结论仍需维护者核验。
| del options | ||
|
|
||
| @functools.wraps(original) | ||
| def wrapper(self, attention_mask, input_tensor, cache_position, past_seen_tokens): |
There was a problem hiding this comment.
P1 · 包装函数签名与替换目标 _update_causal_mask 不匹配，前向调用会抛 TypeError
触发条件： 按 docstring 声明将返回的 wrapper 安装为 LlamaModel._update_causal_mask 后，执行任意一次模型前向。
证据与影响： wrapper 仅接受 attention_mask/input_tensor/cache_position/past_seen_tokens 四个位置参数，而 transformers 各版本 LlamaModel._update_causal_mask 调用形如 (attention_mask, input_tensor, cache_position, past_key_values/past_seen_tokens, output_attentions)，实参多一个，第57行对 original 的透传也缺该参。 启用该优化组后首次前向即抛类型错误，训练/推理中断；单元测试若自造四参调用则无法暴露此问题。
修改建议： wrapper 改用 *args/**kwargs 透传，或与目标签名的位置与语义完全对齐，并用真实 LlamaModel 做一次前向冒烟验证。
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
1 participant
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.
摘要
将 OpenVLA 的 HCU（ROCm/HIP）支持与训练栈优化作为随包 OptimizationConfig 和 RuntimeConfig 提供。用户通过
turbo-physai run启用，训练仍由 OpenVLA 原生入口脚本运行，无需修改 OpenVLA 源码。变更
fsdp-shard-grad-op；新增 AdamW fused、FSDP1 通信重叠、GC 冻结、文本长度定长 padding 等训练栈优化。model_examples/OpenVLA/README.md应用文档。openvla.data.text_len_bucket会略微增加显存占用；openvla.dataloader.spawn会改变有效 batch 流顺序，做数据流对齐实验时需关闭。test/engine/test_generated_configs.py、test/optimizations/test_openvla_skip_fa2_unpad.py、scripts/check_docs.py与git diff --check，均通过。关联 Issue：无。