Repository navigation
Conversation
# ⭐ Feature ## Share the inference control plane - Add the CPU-only gateway, common manager, lifecycle coordinator, placement planner, routing schema, and compatibility client for Rollout, GenRM, and Teacher. - Validate split, defer, decoupled placement and reject same-phase co-residency before startup. - Coordinate drain, offload, onload, deferred Teacher publication, and dynamic Rollout updates without changing role workloads. --- # ✅ Tests ## Cover RFC redai-studio#71 contracts and GPU workflows - Add CPU tests for routing, discovery, placement, lifecycle rollback, PG ownership, static-weight protection, legacy clients, and deferred publication. - Add opt-in PP/PD topology and three-role GPU integration tests and validation launchers. - Record passing split and decoupled multi-node training evidence in the bilingual validation guide. --- # 📝 Documentation ## Document the acceptance evidence - Add English and Chinese API and validation guides with reproducible commands and acceptance mapping. - Keep the scope limited to the explicit RFC redai-studio#71 acceptance checks.
Nyanpasu 审查看板审查状态： ✅ 已通过 审查版本： 0e1b722 目标分支： main 第五轮复查头 0e1b722（较上轮 36ee4ba 新增三笔提交，均针对上轮发现与 CI 反馈）：79d98456 将 engine-pick 回归测试改写为按规范发现快照逐次取准入（旧断言对应集成前缓存语义，36ee4ba8 的 CI 已证实失败）；bf354e70 恢复 GenRM 对全局 sglang_* 参数的继承（与合并基线逐字节一致，F4 随之解决），并以真实 dataclasses schema 重建契约测试，无需安装 SGLang 即可执行，消除原 importorskip 盲区；0e1b7228 隔离 _dep_stubs 的引擎导入桥（try/finally 恢复 sys.modules 与包属性），修复了对未改动测试的模块缓存污染（bf354e70 CPU 测试失败的机制）。本地逐文件复跑 scale/registry/contract/pick/precheck/stubs/sglang/gateway 套件共 224 例加 5 子测试全部通过；bf354e70 的 GPU Unit Tests 与 Integration Tests（Qwen3-4B 4GPU/2GPU/4NPU）经 GitHub API 核实成功。当前头的 CPU Tests 因 bypass 门未开尚未运行，多节点 GPU 验收沿用功能性前驱 bf354e7 的运行结果（与本头仅差测试文件）。结论 APPROVE；上轮发布时 36ee4ba 的 CI 失败（engine-pick 旧测试）未及时纳入结论，本轮已核对并确认由 79d9845 修复。
审查发现待处理
已解决或已取代
提交范围 · 接收 115 · 建议移出 0 · 待确认 0接收 115 个文件 · 建议移出 0 个文件 · 待确认 0 个文件。移出与待确认部分暂停深审，不代表审查通过。
精简审查与验证依据
生产代码的必要性与替代方案
测试的必要性与替代方案
Powered by Nyanpasu with glm-5.3-flash max, please check the suggestions carefully.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
已完成审查，有一项非阻塞的启动就绪问题，详见行级评论。
本地 CPU 契约测试为 91 通过、1 跳过；未发现需要阻塞合并的问题。GitHub 暂无 CI 检查记录，完整训练及多节点 GPU 验证未在本机复跑。
| complete | ||
| and group.lifecycle_status == EngineGroupLifecycle.ACTIVE | ||
| and not self._is_weight_updating | ||
| and self._policy_weights_ready.get(name, False) |
There was a problem hiding this comment.
请在纯 fully-async 的首次权重同步成功后发布就绪状态。包含 Actor 的默认模型初始为 False，但 Controller 启动时调用的 Actor.update_weights_fully_async → MegatronTrainRayActor.update_weights_fully_async 只完成 DCS 传输，没有更新此标志；目前异步路径要等首次训练后的 /end_update_weight 才设为 True。因此首次同步已成功、引擎可用时，/rollout/generate、/rollout/v1/chat/completions 和新客户端仍会返回 503。建议在初始同步确认成功后解除门控，并补充“首次训练前即可通过网关生成”的回归测试；失败同步仍应保持不可用。
There was a problem hiding this comment.
已确认在 4335eab 修复。Actor 在等待全部训练 worker 的初始异步同步成功后发布 Rollout 就绪状态；同步异常和 actor_fwd-only 分支不会发布。已核对启动调用链，并复跑 CPU 契约测试：94 通过、1 跳过。F1 已解决。
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
复查通过：此前的启动就绪问题已修复，本轮增量未发现新的审查问题。CPU 契约测试 94 通过、1 跳过（缺少 Ray）；GitHub 暂无 CI 检查记录，本机未复跑多节点 GPU 验证。
|
@SigureMo 任务已完成，请求 review。 |
SigureMo
changed the title
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
新到达的 CI 结果确认了一项测试替身兼容性回归，详见行级评论；此前 F1 仍已解决。本次补充意见不要求阻塞合并。
Pre-commit 检查 也因 ruff-format、docformatter 修改文件而失败，需要应用格式化结果。GPU 检查仍在运行。
| await self._weight_update_ready.wait() | ||
|
|
||
| self.status = "running" | ||
| await self.rollout_manager.set_policy_weights_ready.remote(True) |
There was a problem hiding this comment.
新增 RPC 后需要同步更新 tests/components/test_rollout_weight_update_handshake.py 的 _ManagerStub。它目前仅提供 health_monitoring_pause 和 set_weight_updating，因此现有 test_end_update_weight_does_not_block_after_failed_can_do 在此处抛出 AttributeError: '_ManagerStub' object has no attribute 'set_policy_weights_ready'。此提交的 Python 3.11 CI 已复现该失败（458 通过、47 跳过后停止）。请给替身补充可等待的就绪 RPC，并保留故障后恢复握手的原有断言；新增的 inference 契约测试没有覆盖这个旧测试调用链。
There was a problem hiding this comment.
已确认 c519b19 修复 F2：测试替身补充了可等待的就绪 RPC，原有故障恢复断言保留。由于本机缺少 Ray，我提取生产方法并隔离 Ray 导入边界，执行该文件的 6 项握手测试，全部通过；CPU inference 契约测试另有 94 通过、1 跳过。完整 CI 仍需以新提交结果为准。
# 🐛 Bug Fix ## Keep weight readiness test doubles compatible - Add the rollout readiness RPC to the handshake manager stub. - Preserve failed synchronization behavior while allowing successful resume. --- # 🎨 Style ## Apply required CI formatting - Apply the repository's ruff and docformatter output to touched Python files.
|
@rai-studio-bot review |
|
@SigureMo 不好意思，只验证了rai-studio-bot，忘记检查CI了，我先修复下CI错误 |
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
复查通过：F1、F2 均已解决，本轮其余变更仅为格式化，未发现新增问题。
CPU 契约测试 94 通过、1 跳过；隔离 Ray 导入边界后的 6 项握手测试通过。新提交暂未显示 CI 结果，本机未复跑多节点 GPU 验证。
| { text: 'Distributed Checkpoint', link: '/zh/guide/distributed-checkpoint' }, | ||
| { text: '健康检查管理器', link: '/zh/guide/health-check-manager' } | ||
| { text: '健康检查管理器', link: '/zh/guide/health-check-manager' }, | ||
| { text: '统一推理服务验收记录', link: '/zh/guide/unified-inference-validation' } |
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
已复核当前提交 c519b19：与上次审查相同，F1、F2 仍已解决，未发现新增代码问题。
Pre-commit 已通过，Python 3.10–3.12 及 GPU 检查仍在运行。此前本地结果为 94 通过、1 跳过，以及隔离 Ray 导入后的 6 项握手测试通过，不能替代完整 CI。
还需回应维护者关于验收记录进入正式文档的意见，确认文档范围；此前代码审查通过不代表该意见已解决或 PR 已满足合并条件。
# 🐛 Bug Fix ## Preserve legacy engine discovery and startup fixtures - Keep `get_engines_info()` compatible with managers created without the unified inference manager. - Update the startup cleanup fixture for the fully async weight-update arguments and metrics state. --- # 📝 Documentation ## Keep the public inference API focused - Remove internal acceptance reports and experiment details from the public API docs. - Move shared inference links into the related lifecycle and placement sections. --- # 🔩 Chore ## Remove private validation wrappers - Delete one-off cluster scripts containing site-specific paths and runtime settings.
There was a problem hiding this comment.
本轮代码兼容性修复及文档清理未发现新增运行时问题；F1、F2 仍已解决。CPU 契约测试 94 通过、1 跳过，另有 4 项隔离的引擎信息兼容检查通过；当前提交 Pre-commit 已通过，Python/GPU CI 仍在运行，本机未复跑多节点 GPU 验证。
guide/unified-inference-validation.md，并称 API 页面包含验收记录和复现信息，但这些段落也已移除。建议把验收结果及复现信息放在 PR 描述或可访问的附件中，删除失效的文件引用，并将剩余 API 页面说明改为接口与用法文档。
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
复查通过：F3 已解决，PR 描述已移除失效引用，并准确说明 API 页面的用途。此前 F1、F2 仍已解决。
代码提交未变化，沿用此前验证结果；Pre-commit 已通过，Python/GPU CI 仍在运行。本审查结论不替代 CI 结果或维护者的合并决定。
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
复查 375ed00a 通过：合并 main 后，算法注册与奖励处理、critic 放置及设备/权重同步接口未发现新增回归；此前 F1–F3 仍已解决。
推理契约测试 94 通过、1 跳过，另有 6 项隔离握手检查通过。Pre-commit 已通过，Python/GPU CI 仍在运行；本机缺少相应运行环境，未复跑多节点 GPU 验证。
|
@SigureMo 任务已完成，请求 review。 |
Re-review P1: the three-state cleanup gate (UNKNOWN/False/True, missing
!= clean) treats every service status response without an explicit
cleanup_required as UNKNOWN, but that flag only existed in GenRM's
status contract. Rollout's ScaleOutStatusResponse and
ScaleInStatusResponse never declared it, so FastAPI's response_model
stripped the key from the wire and the shared autoscaler -- whose
default target is rollout -- read UNKNOWN for every rollout terminal
operation:
- terminal scale-out (ACTIVE/PARTIAL/FAILED/CANCELLED) and scale-in
(COMPLETED/FAILED) stayed pending_requests forever;
- the next scaling evaluation stayed frozen ("in progress or awaiting
cleanup") with no path to unblock;
- a removable rollout-contract target kept failing PATCH removal with
409 (stale unresolved pending);
- history never recorded the finished operation.
Rollout has no deferred-cleanup lifecycle: a FAILED scale-out rolls its
engines back before reporting and a COMPLETED scale-in reports the
engines removed, so terminal status is cleanup-complete by contract.
The fix therefore makes the rollout status models state that explicitly
(cleanup_required: bool = False) instead of leaving the flag absent.
GenRM's semantics are unchanged: a GenRM POST reply still proves
nothing, a missing/malformed flag still reads UNKNOWN, terminal-dirty
still requires status/reconcile, and finalization still requires an
authoritative cleanup_required is False.
Coverage:
- response-model serialization tests pin model_dump()["cleanup_required"]
is False for both scale-out and scale-in wire shapes (the response-
model tests now import the component through tests.utils._dep_stubs
so they actually run where transfer_queue is absent);
- shared-autoscaler regressions drive a real AutoscalerService against
a fake speaking the rollout HTTP contract through the actual Pydantic
response models: scale-out and scale-in terminal requests finalize
into history, the next decision is no longer blocked, and a finalized
target no longer 409s on removal;
- the fail-closed guard stays pinned: a rollout-contract response with
the flag dropped (pre-fix wire shape) keeps the request pending and
the decision frozen -- the fix lives in the contract, never in the
autoscaler defaulting missing to clean.
…ic probe declines Re-review P2: _compute_genrm_server_args pinned attention_backend unconditionally -- including the architecture probe's None -- so the sglang_* inheritance loop (which backfills only fields absent from kwargs) could never apply the user's global --sglang-attention-backend to a GenRM engine again. The docstring's "falls back to SGLang default" only held for SGLang's internal default; the relax-level global flag was silently disabled by 5c1e2e7. The deterministic backend is now pinned only when the user has not chosen one globally: - explicit --sglang-attention-backend wins over the probe (restores the pre-5c1e2e7 inheritance behaviour); - a failed probe defers to SGLang's default instead of pinning None over it (and over the user's global choice); - the arch-appropriate deterministic backend (fa3 on pre-Blackwell, flashinfer on Blackwell+, radix cache preserved) is still pinned when the user set nothing, keeping the final-code autoscaler acceptance behaviour on this machine; - per-instance --genrm-engine-config overrides still take priority over both (applied after the loop). Regressions: - an explicit global choice beats the probe result; - a failed probe pins nothing (key absent, SGLang default applies) while the user's global choice still flows through the inheritance loop.
The module-level transfer_queue attribute guard (added with the rollout contract stub support) places two imports after executable code; ruff's E402 fires on the pushed tree and would fail the Pre-commit workflow. Annotate both imports as guarded.
…tion PASS, r5 proxy incident, train-continuity reanalysis v2) Evidence-branch pointer update only; no product change. Raw artifacts live on the evidence branch @ d88d6c7 (r6/r4 verdict trees, the archived raw job log with sha256, and the reanalysis v2 script + parser tests).
…ons table Per review: the top of the file still pointed at evidence 6b501a2 and called 5c1e2e7 the newest acceptance code while the bottom appended 0481701 reconfirmations — an index that contradicted itself. The top is now the only current-conclusions table: one row per official criterion, each bound to the exact product SHA, the producing run and its raw inputs on evidence 66f1f78, with an explicit superseded-by column. Reading rules state the caliber of each claim (0/50 = these 50 inputs under this config; GenRM-path-only weight sync; criterion 5 is continuity, no performance claim absent a control run; content-deterministic semantics pending maintainer acceptance). A scope commitments table separates delivered / not-implemented / to-be-negotiated (cross-Gateway wording now 'not implemented this phase; interface and responsibility to be negotiated'). The per-run history below is marked as historical detail and keeps its own producing commits.
- docs/en/api/genrm.md + docs/zh/api/genrm.md: endpoints table, request/ status contract, NOOP/idempotency semantics, scale-out/in state chains with terminal sets, cleanup_required and capacity fields (current/ready/ occupied/pending_cleanup), strict-capacity 503 and fallback semantics - docs/public/openapi/genrm.json: regenerate via scripts/tools/ generate_openapi.py; 10 paths, 9 schemas, descriptions from code docstrings - scripts/tools/generate_openapi.py: write trailing newline so reruns are byte-stable Verified against relax/components/genrm.py, relax/utils/genrm_scale_registry.py and relax/distributed/ray/genrm.py: every documented route, state, terminal set, capacity formula and error code matches the implementation.
- docs/public/openapi/rollout.json: add cleanup_required (bool) to ScaleOutStatusResponse and ScaleInStatusResponse properties — manual patch in generator-faithful style (type + title, not in required), avoiding the unrelated /predict drift a full regeneration would bring - docs/en/api/rollout.md + docs/zh/api/rollout.md: new 'Scale status cleanup contract' section (bilingual semantic parity) documenting the three-state contract shared with the Autoscaler (true = cleanup still pending; false = authoritative complete; absent = unknown, missing != clean) and why rollout terminal states report false (scale-out engines serve or were rolled back; scale-in COMPLETED removed / FAILED rolled back) Docs-only; no product code touched.
# 📝 Documentation - Point the two evidence index links to the reviewed fdf288d snapshot. - Leave product code, historical verdicts and experiment versions unchanged. - Keep this commit local; the public PR head and its CI remain unchanged.
# 📝 Documentation - Withdraw whole-run zero-error and stall-bound claims using reanalysis v2.1. - Distinguish log-reported sample counts from file-level sample verification. - Keep original verdicts and producing versions; mark superseded interpretations. - Correct the long-request drain description and completed reconfirmation status. - Preserve product code, GPU results and public PR history.
|
辛苦解决下冲突～ |
好的，我尽快修改 |
Merge main at 7c4e537 while preserving nonblocking rollout manager binding and initial weight synchronization. Apply the upstream offline-mode guard so SFT, DPO, and RM continue to skip startup weight updates. Retain the English and Chinese unified inference navigation entries using the current main formatting. Extend startup regression coverage across online/offline loss modes and synchronous, fully asynchronous, and hybrid execution. Validation: selected CPU regressions passed (110 passed, 1 skipped, 2 deselected); all pre-commit hooks passed. Full Ray and multi-node GPU validation was not rerun in the local environment. Refs redai-studio#71
|
Thanks for contributing to Relax, @Arazyu! 感谢你为 Relax 做出贡献！ 📚 Contribution guide / 贡献指南Describe the problem, your changes, and how you validated them. Keep each PR focused and run 请说明问题、改动和验证方式，保持 PR 聚焦，并在提交前运行 🛠️ CI commands / CI 指令
Put one command on the first line of a new PR comment. Rerun/cancel require PR authorship or repository write access. 在新 PR 评论的首行写一条指令。PR 作者或有仓库写权限的贡献者可以重跑、取消 CI。 🔎 Merge requirements / 合入条件GitHub：⏳ 合入条件未满足
|
|
/review |
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
复查结论：通过（904b4b87）
冲突解决提交已逐项核对：57 个变更文件中 48 个与本轮已通过的 375ed00 的 PR diff 字节一致；其余 9 个文件中，docs/.vitepress/config.mts 保留统一推理中英双导航入口，relax/components/actor.py 将 main 的 SFT/DPO/RM offline-mode 守卫（is_offline_mode）并入本 PR 的非阻塞启动同步结构，配套启动测试改用真实守卫并把参数化扩到 12 组，其余仅行号偏移。F1–F3 的修复在新头均保留；本地 104 项 inference 契约测试与 15 项握手/运行时测试通过，该提交 CI 全绿。多节点 GPU 验证未在本机复跑，沿用 PR 描述中的验收记录。
说明：当前合并因维护者此前的 changes-requested 决定而阻塞；其中验收记录页的意见已由该页移除落实，是否放行由维护者复核决定。
The NPU provider copies runtime arguments into the Bridge model config. The inference placement plan contains frozen dataclasses, which the NPU image's older Bridge sanitizer tries to rebuild with setattr during the initial weight export, raising FrozenInstanceError on the role field. Keep the placement plan on the runtime args and exclude it from the model config. Preserve the other NPU argument copies and existing provider overrides. Add GPU/NPU provider regressions that retain the original plan. Validation: the NPU regression fails before the fix; the provider and inference suites pass afterward (123 passed, 1 skipped because Ray is unavailable). All pre-commit hooks pass. Real NPU validation remains with the integration CI. Refs redai-studio#71
|
/review |
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
复查结论：通过（9eec00a1）
新提交相对上轮已通过的 904b4b8 仅新增两个文件（+25 行）：relax/backends/megatron/model_provider.py 在 NPU 分支整体拷贝 vars(args) 时排除运行时放置计划 _inference_placement，配套回归测试用真实 PlacementPlanner 构造放置计划并参数化 NPU 可用性。已核对该字段唯一写入方为 relax/inference/placement.py:171，NPU 拷贝循环是唯一会把该 frozen dataclass 计划带上 provider（随后被整体 pickle）的路径；排除后 args 仍持有计划供运行时消费，无其他泄漏面。本地 20 项 provider 测试与 104 项 inference 契约测试通过，该提交 CI 全绿。其余 57 个变更文件与上轮已通过内容一致，结论沿用；F1–F3 修复均保留。多节点 GPU 验证未在本机复跑，沿用 PR 描述中的验收记录。
说明：当前合并因维护者此前的 changes-requested 决定而阻塞；其中验收记录页的意见已由该页移除落实，是否放行由维护者复核决定。
Merge the GenRM elasticity implementation with the unified inference service. Preserve legacy generation, sampling and attribution through the gateway, count raw and streaming requests during drain, and bind closed admission to engine identities so recovered engines may reuse endpoints. Use the same published engine states for discovery and legacy routing. Preserve split placement offsets, retain owned placement groups until Ray confirms removal, and recreate elastic placements during engine recovery. Exclude retired slots from recovery and keep failed cleanup retryable. Reject deferred, training-offload and multi-GPU elastic layouts before taking the lifecycle lock or allocating resources. Document the supported scope in both languages and regenerate the GenRM API schema. Validation: focused CPU suite 426 passed, 4 optional-dependency skips; latest lifecycle/gateway regression rerun 34 passed (overlaps that suite); additional contract tests 12 passed, 17 optional-dependency skips. All pre-commit hooks and the VitePress documentation build passed. Real SGLang and multi-node GPU execution were not rerun on this CPU host. Refs redai-studio#71, redai-studio#351, redai-studio#356, redai-studio#370
|
/review |
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
复查结论：有一项非阻塞回归（P2）
本轮增量是提交 36ee4ba 把 #370 的 GenRM 弹性扩缩容并入本 PR。已逐块核对集成提交对网关、放置与 Teacher 共享语义的兼容改造：legacy /generate 与网关原生请求共用同一 in-flight 计数与 drain 证明命名空间；生命周期操作通过非阻塞 _exclusive_lifecycle 与弹性扩缩容互斥，发现快照不持转移锁、扩缩容期间发现服务仍可用；弹性 PG 保留至 Ray 确认 REMOVED，失败清理保持 fail-closed 并可重试。本地 CPU 套件 330 例：328 通过、2 失败，失败项即下方行级评论的 P2——并入的 #370 测试断言 GenRM 继承全局 sglang_* 参数，而该继承循环在本 PR 早期提交中已从 _compute_genrm_server_args 删除；CI 环境缺少真实 sglang 而跳过这两条测试，未能暴露。F1–F3 修复均保留；#370 引入的弹性扩缩容本体沿其在 #370 的审查讨论评估，此处不复述。
说明：多节点 GPU 与真实 SGLang 运行未在本机复跑，沿用 PR 描述披露的验收边界；当前合并因维护者此前的 changes-requested 决定而阻塞，是否放行由维护者复核决定。
| # (re-review finding). A failed probe defers to SGLang's default instead | ||
| # of pinning ``None`` over it; per-instance ``--genrm-engine-config`` | ||
| # overrides still take priority over both (applied after the loop). | ||
| if getattr(args, "sglang_attention_backend", None) is None: |
There was a problem hiding this comment.
合并基线（7c4e5371）的 _compute_genrm_server_args 内含 sglang_* 继承循环；本 PR 在 a3bbb30 删除了其中两行（if hasattr(args, f"sglang_{attr.name}") and attr.name not in kwargs: 及其赋值，见 hunk @@ -1632,8 +1699,6），而 #370 并入时新增的注释（"the sglang_* inheritance loop below backfills only fields absent from kwargs … (applied after the loop)"）与两条测试仍按该循环存在来断言。
本地装有真实 sglang 0.5.17 的环境复跑 tests/backends/sglang/test_sglang_engine.py：test_genrm_attention_backend_respects_global_user_choice 与 test_genrm_attention_backend_probe_failure_defers_to_global_and_default 均失败（用户显式 sglang_attention_backend 未出现在 kwargs）；CI 环境因缺少真实 sglang 由 importorskip 跳过，故未能暴露。rollout 侧 _compute_server_args 的同名循环保留完好，两侧行为已分叉。
影响：用户显式 --sglang-attention-backend（以及其他未在 kwargs 显式计算的全局 sglang_* 项）对 GenRM 引擎静默失效；此时确定性推理落到 SGLang 通用默认后端，即上方注释所述 radix cache 被禁、48 路并发评分预填楔死场景的入口。
建议：恢复 genrm 函数内被删的两行继承循环（保留当前 guard "仅在用户未显式选择时探测" 的语义），或在文档中明确 GenRM 不继承全局 sglang_* 并同步修正此注释与两条测试；修改 relax/utils/arguments.py 或后端参数语义需按仓库约定先征求维护者确认。
There was a problem hiding this comment.
已在 bf354e7 确认修复：`compute_genrm_server_args` 恢复了被删的两行 `sglang*` 继承循环，与合并基线 1632–1637 行逐字节一致（rollout 侧同构，两侧行为不再分叉）。原先两条依赖真实 SGLang 的测试恢复通过，且 tests/inference/test_engine_contract.py 已改用真实 dataclasses schema 直接执行生产构建器（31 例通过，不要求安装 SGLang），原先 CI 因 `importorskip` 跳过而无法暴露此回归的盲区随之消除。另：bf354e70 的 CPU 测试失败源于 `_dep_stubs` 导入桥的桩模块泄漏污染 `sys.modules`，已在 0e1b722 隔离修复，与本条发现无关。
Exercise canonical discovery on every engine pick while preserving round-robin, recovery and lifecycle admission coverage. Inject watcher failures at the processing boundary and test repeated registry eviction with small caps instead of long waits and large scans. Validation: 444 passed, 4 dependency skips in focused CPU regressions; 36 registry tests passed after strengthening protected-record eviction coverage; all pre-commit hooks passed.
Backfill supported global SGLang fields without replacing GenRM role defaults, then apply per-model overrides. Add CPU-only builder regressions for attention precedence, memory tuning, static-model defaults and the decode cache exclusion so missing SGLang cannot hide this regression. Validation: 31 engine contract tests passed; 454 focused tests passed with 4 dependency skips; all pre-commit hooks passed.
Restore both the SGLang engine module cache entry and its parent-package attribute after importing the GenRM manager, including failed imports. This prevents later CPU tests from mistaking a dependency bridge for an installed engine runtime. Validation: 9 import-isolation regressions passed; ordered manager/precheck regression passed 31 tests with 20 missing-SGLang skips; all pre-commit hooks passed before this commit.
|
/review |
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
复查结论：通过（APPROVE）
本轮增量（36ee4ba8..0e1b722，三笔提交）均已逐项核实：79d98456 将 engine-pick 回归测试改写为按规范发现快照逐次取准入——旧断言对应集成前的缓存列表语义，且该头 36ee4ba 的 CI 失败（get_inference_snapshot 缺失）证实其已失效，watcher 测试改为在处理边界注入异常并断言 poll/sleep 次数，registry 洪水测试限界后覆盖等价并加强驱逐断言；bf354e70 恢复 _compute_genrm_server_args 对全局 sglang_* 的继承（与合并基线 1632–1637 行逐字节一致，rollout 侧同构），F4 随之解决，契约测试改用真实 dataclasses schema，不安装 SGLang 即可执行，消除了原先 importorskip 造成的 CI 盲区；0e1b7228 以 try/finally 恢复 _dep_stubs 引擎导入桥的 sys.modules 与包属性绑定，修复了 bf354e7 CPU 测试失败的模块缓存污染机制。本地逐文件复跑共 224 例加 5 子测试全部通过；bf354e70 的 GPU Unit Tests 与 Integration Tests（Qwen3-4B 4GPU/2GPU/4NPU）经 GitHub API 核实成功，该头与本头仅差测试文件。当前头的 CPU Tests 因 bypass 门未开尚未运行，待其完成后如有失败由后续跟进处理。说明：上轮在 36ee4ba 发布时该头 CI 已有 engine-pick 旧测试失败而未及时纳入结论，本轮已核对并确认由 79d9845 修复；多节点 GPU 完整训练流程未在本机复跑，沿用 PR 描述披露的验收边界。
已解决冲突 |
This branch has not been deployed
6 participants
Summary
Implements the RFC #71 unified inference service scope for Rollout, GenRM, and Teacher, with integration of GenRM elastic scaling from #370.
InferenceGateway, sharedInferenceManager/lifecycle, placement planner, routing schema, and compatibility client.GenRM integration
Commit
36ee4ba8edca29fcf6d1d563eaabc503842b5512integrates #370 atda4acbbab530f082f37c1633c811c704be5a044einto the previous #356 head,9eec00a186b4b92f32dc2aba9c23e673e2375b13. This PR now includes #370's implementation and original commit history, together with the compatibility fixes:REMOVED, reconcile unfinished cleanup, and create a fresh PG when recovering a failed elastic engine.--offload-rollout) and multi-GPU elastic engines before taking the training lifecycle lock or allocating resources.sglang_*tuning inheritance without overriding GenRM role defaults; per-model engine configuration keeps priority. Cover this contract in CPU tests that do not require SGLang.Elastic scaling retains #370's scope: single-GPU elastic engines and drain accounting within one Gateway. Consistent direct-client routing does not establish direct-client or cross-Gateway graceful drain. Pools that participate in training offload do not support live elastic scaling in this integration.
Acceptance evidence
The following are previously reported results from before the integration with #370. They have not been rerun after integration with #370 and do not establish GPU acceptance of the integrated head:
6654867: PP/PD routing and complete Megatron data flow passed.6772050: four-node split training passed, including Teacher writeback before GenRM and completed Megatron steps.6780723: four-node decoupled training passed, including Teacher results before training and dynamic Rollout updates.Evidence inherited from #370 and
demos/task4_genrm/likewise belongs to its original producing revisions, not a new integration run.The public API pages document the unified inference interfaces and usage:
docs/en/api/inference.mddocs/zh/api/inference.mdValidation
Current head:
0e1b7228bf3f058e9cca8e16bdea43357f9f2e5d.The current follow-up isolates the test-only SGLang import bridge: module-cache and parent-package bindings are restored on both success and failure, while an available runtime is imported unchanged. The isolation regressions passed 9 tests; the combined manager/precheck run passed 31 tests with 20 missing-SGLang skips and 5 passing subtests. These counts overlap. All pre-commit hooks passed before committing. The current head requires fresh remote CI results.
Remote validation on the preceding functional head
bf354e7:The following functional regression results were also collected on
bf354e7. Local environment: macOS arm64, Python 3.12.13, Ray 2.59.0, pytest 9.1.1, CPU only. Python commands ran throughuvin a temporary test environment.pre-commit run --all-filesThe four skips concern an unavailable Rollout import. The suite uses the existing test-only import bridges for unavailable optional dependencies; the argument regressions execute the production builder with a minimal supported
ServerArgsschema and do not require an installed SGLang runtime. Broader local CPU testing remains incomplete because this macOS environment lacks some Linux/runtime dependencies; it is not reported as a full-suite pass. Multi-node GPU acceptance has not been rerun after the integration; remote CI results must be assessed on the current head separately.The GenRM routing tests now consume canonical discovery snapshots and verify immediate admission changes, round-robin continuity and recovery. The watcher regression injects the exception at the processing boundary without a real deadline wait. Registry tests cross reduced test-only capacity limits repeatedly and verify eviction while protecting live and dirty records; production limits and CI timeouts are unchanged.
The VitePress documentation build passed on integration commit
36ee4ba; the subsequent fixes do not modify documentation.Test commands
TEST_PYTHONis the Python executable in the temporary test environment.Refs #71, #351; integrates #370.