Conversation
# 🔧 CI/CD ## Run unit tests on host GPU runners - Trigger GPU unit tests automatically for pull requests and pushes to main. - Use the public latest image with a fixed four-GPU allocation per runner, a shared host uv cache, and a private container workspace. - Keep dependency installation and the complete pytest run in separate workflow steps, preserving test failures and existing skips. - Always remove the container and upload pytest logs and JUnit results. Co-authored-by: Codex <noreply@openai.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Critical runner-isolation issues and known failing tests must be addressed.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a self-hosted H20 GPU unit-test workflow for pull requests, main pushes, and manual runs.
Changes:
- Runs containerized GPU tests with uv dependency installation.
- Uses fixed GPU allocations and shared caching.
- Cleans up containers and uploads pytest artifacts.
File summaries
| File | Review summary |
|---|---|
.github/workflows/unittest.yml |
Adds the GPU workflow and artifact handling. Critical concerns remain around executing untrusted PR code on persistent self-hosted runners and shared host state; the known six test failures also keep the workflow red. |
Review details
Suppressed comments (1)
.github/workflows/unittest.yml:85
- The command intentionally propagates pytest's exit status, but the PR's reported run already has six failures in
tests/backends/megatron/test_chunked_mtp_loss.py. As written, every PR andmainrun will remain red, so this cannot be an automatic unit-test gate until that baseline is fixed or the workflow is deliberately prevented from blocking merges.
python -m pytest tests/ -v -ra --tb=short --junitxml=/artifacts/junit.xml \
2>&1 | tee /artifacts/pytest.log
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| pull_request: | ||
| branches: [main] |
| jobs: | ||
| unittest: | ||
| name: Unit Tests (H20, 4 GPUs) | ||
| runs-on: [self-hosted, linux, x64, h20, gpu] |
There was a problem hiding this comment.
补充核验：这个 fork PR 的首次运行已经在 H20 runner 上完成 Checkout 并进入宿主机的 Create test container 步骤；缺少 CI_GPU_DEVICES 只会让当前脚本退出，并不是执行权限边界。PR 若修改这些 run 步骤，就能以 runner 账户直接使用 Docker，绕过第 49–64 行设置的 GPU、网络与挂载限制，影响同机另一 runner 及后续任务。
建议在启用公开 PR 自动执行前，把整个 runner 放进每次销毁的隔离环境；若保留持久化主机，则需要在 PR 无法修改的调度/runner 访问策略中限制为受信工作流和经审核的提交，单独增加本 YAML 内的条件不足以建立边界。GitHub 的自托管 runner 安全说明也指出了这种持久化风险。当前账户无法读取仓库级 fork 审批策略，因此这里不声称所有外部 PR 都会免审批运行；如已有外部约束，请提供其配置依据以便复核。
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db2f18ebd7
ℹ️ 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".
| pull_request: | ||
| branches: [main] |
There was a problem hiding this comment.
For a fork PR, the checkout contains contributor-controlled code that is executed by pytest on the persistent self-hosted H20 runner. The container still receives the runner's proxy values and writable shared uv cache, so malicious tests can exfiltrate embedded proxy credentials or poison state consumed by later trusted jobs; contents: read does not protect these host resources. Require trusted approval or an allowlist before scheduling fork code, or use an ephemeral isolated runner.
Useful? React with 👍 / 👎.
| runs-on: [self-hosted, linux, x64, h20, gpu] | ||
| timeout-minutes: 30 | ||
| env: | ||
| CI_IMAGE: ghcr.io/redai-studio/relaxrl:latest |
There was a problem hiding this comment.
Move this registry/image value—and the package-index endpoint on line 79—into repository or runner configuration rather than baking deployment-specific endpoints into the workflow; the current latest reference also lets the test environment change without a reviewed commit, making failures irreproducible. Pin the configured image by digest while preserving the logged image ID.
AGENTS.md reference: AGENTS.md:L69-L74
Useful? React with 👍 / 👎.
Nyanpasu 审查看板审查状态： 🚧 需要修改 审查版本： 再次核查 CI（claude 后端）：head 与最近的 GPU 运行未变，仍为 2525 通过、6 项 test_chunked_mtp_loss.py 逐位相等断言失败、18 跳过。本轮把根因钉定：失败与 chunk 尺寸是否整除 S=16 完全对应——16、2 整除者通过，3、7 及 chunk_size=3 的用例不整除而失败，最大绝对差 3.7e-9 至 1.1e-8，属末块 GEMM/CE 形状变化引入的 fp32 舍入差，并非 chunked-MTP patch 的逻辑缺陷；建议改为容差比较并修正 BIT-FOR-BIT 措辞（详见本轮最新 PR 评论）。本 CI-only PR 未改动 patch 与测试，六项失败为既有断言、不构成新增阻塞项。原 F1（持久化 runner 信任边界）仍未解决。
Powered by Nyanpasu with claude-opus-5-google[1m] medium, please check the suggestions carefully.
|
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
本轮暂不通过：持久化 H20 runner 的执行信任边界尚未得到保证，已在原讨论补充运行证据和修复建议。
已审查全部变更；本地 Shell 语法及失败退出码传递验证通过，现有 CPU CI / pre-commit 通过。GPU 检查正在重跑；本地未执行 GPU / 多节点集成测试，因为审查环境未配置相应硬件。PR 中报告的六项既有 MTP 断言失败未作为本轮新增阻塞项。
|
@rai-studio-bot 帮忙分析下 CI 失败问题 |
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
针对 CI 失败分析请求，已核对 db2f18e 的最新 GPU 作业。这次容器创建、依赖安装、清理和结果上传均成功，失败发生在 pytest：2525 passed、6 failed、18 skipped，耗时 378.40 秒。首次运行缺少 CI_GPU_DEVICES 的问题已不再阻塞这次运行，测试日志与 JUnit已保留。
六项失败全部位于 test_chunked_mtp_loss.py：
| 用例 | 失败断言 / 已打印的最大绝对差 |
|---|---|
test_chunked_mtp_matches_original[3] / [7] |
第 133 行 torch.equal；分别 3.73e-9 / 1.86e-9 |
test_scale_logits_fn_per_chunk |
第 148 行 torch.equal |
test_calculate_per_token_loss_branch |
第 158 行 torch.equal |
test_is_training_logging_matches |
第 197 行 torch.equal |
test_sequence_parallel_gathers_hidden_once |
第 420 行 torch.equal；1.12e-8 |
最可能的原因是测试要求分块与整块浮点计算逐位相等。 这些用例实际使用 CPU fp32 的 mock head（矩阵乘法）及交叉熵；分块会改变运算形状，数学等价不保证逐位一致，参见 PyTorch 数值精度说明。当前日志不能进一步确定差异首先出现在 matmul、CE 还是后续计算。
其中 “gradient tensor mismatch” 比较的是 spy 捕获的前向 loss，并未调用 backward()；SP 用例的 gather 次数和 flag 恢复断言已通过。因此这些报错不能直接解释为真实梯度、NCCL 或 H20 故障。该 PR 没有修改相关测试和实现，失败集合也与 PR 描述中的预跑结果一致。
建议仅调整 full-vs-chunk 的浮点值比较，并修正测试说明中的 BIT-FOR-BIT 承诺；保留层数、shape、mask 和 SP 行为等精确断言。例如以此容差作为同镜像验证的起点：
torch.testing.assert_close(g, r, rtol=1e-5, atol=1e-7)先在本次 CI 镜像单跑 python -m pytest tests/backends/megatron/test_chunked_mtp_loss.py -v -ra，记录 logits、CE、归一化 loss 的绝对/相对误差，确认容差和后续 logging 断言，再跑全套。CPU CI 为绿色不代表这六项已通过：CPU 日志没有这些用例的执行记录，而该文件在 Megatron/Relax 导入失败时会整模块 skip。
本轮是诊断，未修改代码或重跑远端任务；本地 PyTorch 2.1 的简化 CPU 探针未复现差异，尚未在相同镜像验证修复。此前 runner 信任边界审查结论保持不变。
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment was marked as duplicate.
This comment was marked as duplicate.
1 similar comment
|
@rai-studio-bot claude code backend 测试，你再看看 CI 问题呢 |
|
已按请求再次核查 这次比对日志后可以把根因钉得更死：失败与 chunk 尺寸是否整除序列长度
也就是说，当末块尺寸与其余块不同（ 结论与此前 CI 分析一致：本 PR 只新增 workflow，未改动 patch 与测试，六项失败是既有断言、与 PR 描述预跑一致，不构成本次 CI-only PR 的新增阻塞项。 修复建议不变——改为容差比较，并同步修正文件头与用例里 torch.testing.assert_close(g, r, rtol=1e-5, atol=1e-7)请注意生产的 chunk 尺寸一般不会整除序列长度，因此正确做法是放宽浮点比较，而非仅挑选能整除的 chunk。建议先在同一 CI 镜像单跑
Powered by Nyanpasu with claude-opus-5-google[1m] medium, please check the suggestions carefully.
|
What
Add a GPU unittest workflow that runs automatically for PRs targeting
mainand pushes tomain, with a manual trigger for debugging.Why
Run the full unit suite in the public Relax image on the existing H20 host runners, using the same dependency installation as interactive debugging.
How
/workspace/Relaxand shownvidia-smi, install requirements, run pytest, clean up, and upload results.ghcr.io/redai-studio/relaxrl:latestand log the actual image ID. Each runner supplies its fixed four-GPU allocation throughCI_GPU_DEVICES; the container uses 32 GiB of shared memory.uv pip install --system --break-system-packagesand the Tsinghua extra index.always()to remove the container and upload pytest logs/JUnit results.Runner setup
Before enabling the workflow, set each runner installation's
.envand restart that runner:self-hosted,linux,x64,h20,gpu.CI_GPU_DEVICES=0,1,2,3for the first runner;CI_GPU_DEVICES=4,5,6,7for the second runner on the same eight-GPU host.http_proxy,https_proxy, andno_proxyas needed. The workflow forwards them into the container; image pulls use the Docker daemon's proxy.~/.cache/uv(or$XDG_CACHE_HOME/uv). SetCI_UV_CACHE_DIRto override the shared host path.Testing
pre-commit run --all-files --show-diff-on-failure: all hooks passed in Linux, including clang-format and gitleaks. The pinned clang-format binary cannot run on the local ARM Mac.bash -nfor the workflow shell blocks.tests/backends/megatron/test_chunked_mtp_loss.py; they are deliberately left for a separate PR. Existing skips are unchanged.The first GitHub run successfully scheduled on the H20 runner and stopped before container creation because
CI_GPU_DEVICESis not yet set in that runner environment. After completing the runner setup above, the check is expected to fail on the known MTP assertions until they are addressed. This PR does not modify framework code, dependencies, or tests.Type of Change