[CI] guard initial actor/ref KL checks under R3 (routing replay) - #93
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds guards to bypass initial actor/reference zero-KL and KL loss checks in CI when routing replay is enabled, as the actor and reference paths route differently under this mode. The feedback suggests checking the base flag use_routing_replay instead of use_rollout_routing_replay in both data.py and model.py to ensure that tests running with only --use-routing-replay do not trigger false assertion failures.
| if ( | ||
| rollout_id == 0 | ||
| and not getattr(args, "ci_disable_kl_checker", False) | ||
| and not getattr(args, "use_rollout_routing_replay", False) | ||
| and "rollout/log_probs" in reduced_log_dict | ||
| and "rollout/ref_log_probs" in reduced_log_dict | ||
| ): |
There was a problem hiding this comment.
The PR description mentions that the false failures occur under --use-routing-replay (R3), which is exercised by tests like Qwen3-30B-A3B. However, the guard here checks use_rollout_routing_replay instead of use_routing_replay.
If a test is run with --use-routing-replay but without --use-rollout-routing-replay, use_rollout_routing_replay will be False, causing this guard to evaluate to True and the assertion to still run and fail.
Since use_routing_replay is the base flag and is automatically set to True when use_rollout_routing_replay is enabled (via args.use_routing_replay = True in slime/utils/arguments.py), checking use_routing_replay instead will correctly cover both routing replay modes.
| if ( | |
| rollout_id == 0 | |
| and not getattr(args, "ci_disable_kl_checker", False) | |
| and not getattr(args, "use_rollout_routing_replay", False) | |
| and "rollout/log_probs" in reduced_log_dict | |
| and "rollout/ref_log_probs" in reduced_log_dict | |
| ): | |
| if ( | |
| rollout_id == 0 | |
| and not getattr(args, "ci_disable_kl_checker", False) | |
| and not getattr(args, "use_routing_replay", False) | |
| and "rollout/log_probs" in reduced_log_dict | |
| and "rollout/ref_log_probs" in reduced_log_dict | |
| ): |
| if ( | ||
| accumulated_step_id == 0 | ||
| and not getattr(args, "use_rollout_routing_replay", False) | ||
| and "train/kl_loss" in log_dict | ||
| ): |
There was a problem hiding this comment.
Similar to the change in data.py, this guard should check use_routing_replay instead of use_rollout_routing_replay to ensure that tests running with --use-routing-replay alone do not trigger false failures.
| if ( | |
| accumulated_step_id == 0 | |
| and not getattr(args, "use_rollout_routing_replay", False) | |
| and "train/kl_loss" in log_dict | |
| ): | |
| if ( | |
| accumulated_step_id == 0 | |
| and not getattr(args, "use_routing_replay", False) | |
| and "train/kl_loss" in log_dict | |
| ): |
Backports slime #1987 + #1990. Under --use-routing-replay (R3), the actor forward replays rollout routing while the reference forward uses normal routing, so their log-probs / KL are not expected to match bit-for-bit at rollout_id/step 0. The unconditional zero-KL CI asserts therefore raise false failures on any routing-replay + --ci-test run. - data.py: gate the rollout_id==0 log_probs vs ref_log_probs assert on (not use_rollout_routing_replay) and (not ci_disable_kl_checker). - model.py: gate the step-0 train/kl_loss assert on (not use_rollout_routing_replay). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai>
7fdc4e4 to
d3b0425
Compare
…ult-tolerance docs Mechanical / docs sweep for the slime→vime sync (mega-PR C additions, on top of the cherry-picked #1939+#1940 / #1942 / #1943): - slime #1980: fix FLOPs accounting for non-MLA attention. Gate the MLA-shaped qkv/attention flop formulas on `multi_latent_attention` instead of inferring MLA from q_lora_rank/qk_pos_emb_head_dim/v_head_dim (which misattributed flops for non-MLA models). vime/utils/flops_utils.py. - slime #1967: fix PYTHONBUFFERED=16 typo -> PYTHONUNBUFFERED=1 across 12 scripts + command_utils.py (both name and value, matching slime canonical). - slime #1938: guard `vllm_speculative_config` read in _compute_spec_metrics with getattr(...) so --debug-train-only (skip_vllm leaves the attr unset) doesn't AttributeError. vime/ray/rollout.py. - slime #1988: rewrite docs/en/advanced/fault-tolerance.md (scope, health checks, debug/replay path, production pattern), rebranded sglang→vllm / slime→vime, /health_generate→/health, link vllm-config.md. Dropped from this PR after source review: - #1987/#1990 (R3 ci ref/logprob+kl guards) — already in main via vime #93 (b1009dd "Backports slime #1987 + #1990"). - #1974/#1989 — touch examples/coding_agent_rl + its README bullet; that feature is not in vime yet (lands with the agent mega-PR). - #1975 — sglang conda-ci image resolution + a test vime already has + build_conda micromamba fix; no real version-file port for vime. Deferred to after the rollout-data-model PR: #1986/#1985 (they edit that PR's tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai>
…ult-tolerance docs Mechanical / docs sweep for the slime→vime sync (mega-PR C additions, on top of the cherry-picked #1939+#1940 / #1942 / #1943): - slime #1980: fix FLOPs accounting for non-MLA attention. Gate the MLA-shaped qkv/attention flop formulas on `multi_latent_attention` instead of inferring MLA from q_lora_rank/qk_pos_emb_head_dim/v_head_dim (which misattributed flops for non-MLA models). vime/utils/flops_utils.py. - slime #1967: fix PYTHONBUFFERED=16 typo -> PYTHONUNBUFFERED=1 across 12 scripts + command_utils.py (both name and value, matching slime canonical). - slime #1938: guard `vllm_speculative_config` read in _compute_spec_metrics with getattr(...) so --debug-train-only (skip_vllm leaves the attr unset) doesn't AttributeError. vime/ray/rollout.py. - slime #1988: rewrite docs/en/advanced/fault-tolerance.md (scope, health checks, debug/replay path, production pattern), rebranded sglang→vllm / slime→vime, /health_generate→/health, link vllm-config.md. Dropped from this PR after source review: - #1987/#1990 (R3 ci ref/logprob+kl guards) — already in main via vime #93 (b1009dd "Backports slime #1987 + #1990"). - #1974/#1989 — touch examples/coding_agent_rl + its README bullet; that feature is not in vime yet (lands with the agent mega-PR). - #1975 — sglang conda-ci image resolution + a test vime already has + build_conda micromamba fix; no real version-file port for vime. Deferred to after the rollout-data-model PR: #1986/#1985 (they edit that PR's tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai>
…80/#1967/#1938/#1988) (#138) * [CI] add reward-fn cpu tests + cpu-on-main trigger (port slime #1939+#1940) Port of THUDM/slime#1939 (add more cpu ci) + #1940 (run cpu test on main): - tests/test_rm_{deepscaler,f1,gpqa,math,math_dapo}.py: pure-cpu unit tests for vime/rollout/rm_hub reward fns (107 tests, validated locally). - vime/utils/misc.py: lazy 'import ray' so cpu-only paths (rm_hub scoring, plugin contracts) don't require ray. - pr-test.yml.j2: register the 5 rm tests in the cpu (num_gpus:0) matrix; enable push-to-main trigger gated to cpu jobs only (GPU/self-hosted jobs stay PR/label-gated so push never burns the fleet). slime #1939 also added tests/test_sample.py, which references Sample.rollout_id (introduced by the #1926-chain / vime PR #119) — that test is stacked on #119 instead of here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * [Doc] sync customization/trace/pd-disagg docs (port slime #1942) Port of THUDM/slime#1942 (update docs). Brought the technical doc updates (customization.md agentic-workflows section, trace.md, pd-disaggregation.md, en+zh), rebranded slime->vime / sglang_rollout->vllm_rollout / SGLang->vLLM. Divergence handling: - README.md / README_zh.md: kept vime's version (slime's changes were SGLang-branded architecture text + 'Projects built upon slime' marketing + an agentic-examples list — vime uses vLLM and trimmed examples in #126). - Dropped the broken examples/search-r1 link from the added customization text (trimmed in vime); kept examples/multi_agent + examples/fully_async (exist). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * [Chore] remove redundant replay tool + harden unwrap_model import (port slime #1943) Port of THUDM/slime#1943 (remove redundant file): - remove tools/replay_openai_jsonl.py (redundant). - vime/backends/megatron_utils/model.py: make unwrap_model import resilient to the Megatron move from megatron.core.utils to megatron.core.pipeline_parallel.utils (try/except fallback). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * [Sync][C] FLOPs MLA fix, PYTHONUNBUFFERED typo, spec-config guard, fault-tolerance docs Mechanical / docs sweep for the slime→vime sync (mega-PR C additions, on top of the cherry-picked #1939+#1940 / #1942 / #1943): - slime #1980: fix FLOPs accounting for non-MLA attention. Gate the MLA-shaped qkv/attention flop formulas on `multi_latent_attention` instead of inferring MLA from q_lora_rank/qk_pos_emb_head_dim/v_head_dim (which misattributed flops for non-MLA models). vime/utils/flops_utils.py. - slime #1967: fix PYTHONBUFFERED=16 typo -> PYTHONUNBUFFERED=1 across 12 scripts + command_utils.py (both name and value, matching slime canonical). - slime #1938: guard `vllm_speculative_config` read in _compute_spec_metrics with getattr(...) so --debug-train-only (skip_vllm leaves the attr unset) doesn't AttributeError. vime/ray/rollout.py. - slime #1988: rewrite docs/en/advanced/fault-tolerance.md (scope, health checks, debug/replay path, production pattern), rebranded sglang→vllm / slime→vime, /health_generate→/health, link vllm-config.md. Dropped from this PR after source review: - #1987/#1990 (R3 ci ref/logprob+kl guards) — already in main via vime #93 (b1009dd "Backports slime #1987 + #1990"). - #1974/#1989 — touch examples/coding_agent_rl + its README bullet; that feature is not in vime yet (lands with the agent mega-PR). - #1975 — sglang conda-ci image resolution + a test vime already has + build_conda micromamba fix; no real version-file port for vime. Deferred to after the rollout-data-model PR: #1986/#1985 (they edit that PR's tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * docs(mega-C): faithfully port slime #1942 docs (fault-tolerance/pd-disagg/trace) Audit (一模一样): mega-C's original #1942 doc port was incomplete — several docs were stubs or truncated vs slime. Port them faithfully (modulo sglang→vllm): - fault-tolerance.md en: add trailing (trainer-failure note + Related Docs). - fault-tolerance.md zh: was a 13-line stub → full 76-line port. - pd-disaggregation.md en+zh: were 7-line stubs → full 87-line port. - trace.md en+zh: port slime's build_meta example as build_vllm_meta_trace_attrs(output) (vime's real signature). Translation: SGLang→vLLM, /health_generate→/health (vime's real endpoint), sglang-config→vllm-config, `sglang:`→`vllm:` YAML key (vime vllm_config.py requires the 'vllm' key), server→engine; `--prefill-num-servers` flag preserved. All 6 files now match slime line counts (76/76, 87/87, 119/119). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(mega-C): port slime #1942 customization gaps (multi-sample return + teacher_log_probs) Surgically add the two slime #1942 additions that mega-C missed, keeping vime's legitimate divergences (vllm_rollout path, VIME_CONTRACT_* env, search-r1 removed in #74): - "#### Returning multiple training samples for one prompt" section (en+zh): custom_generate may return list[Sample] with shared group_id; signature updated to `-> Sample | list[Sample]`. - `teacher_log_probs` field in the Sample-fields dict (en+zh). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * [Sync][C] align to slime exactly: drop #1961 over-port, fix #1938/#1943 placement - customization.md (en+zh): revert custom_generate signature to `-> Sample` and remove the "Returning multiple training samples" section. That content is from slime #1961 (not #1942, despite the prior commit message) and documents Sample.group_id / Sample.rollout_id + list[Sample] return — all tied to the deferred rollout-data-model refactor that vime does not yet implement (verified: 0 occurrences of group_id/rollout_id in types.py and the rollout loop). Defers with #1986/#1985. - model.py: move the unwrap_model try/except to slime #1943's exact position (after the tqdm import) — import block now byte-identical to slime. - rollout.py: drop the 4-line comment around the spec-metrics getattr guard; slime #1938 is a pure one-line change with no added comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * [Sync][C] cleanup sglang/dead-link residue: /health_generate help text + search-r1 refs - arguments.py: --rollout-health-check-{interval,timeout} help text said `/health_generate` (sglang's endpoint) but the code hits `/health` (vllm_engine.py). Fix the help strings to `/health`. - customization.md (en+zh) + index.rst (en+zh): remove the dead `examples/search-r1` references — that example does not exist in vime. Note: the method name `VLLMEngine.health_generate()` (vllm_engine.py) and its caller (health_monitor.py) keep the sglang-flavored name but already hit `/health`; renaming the method is a separate change, left out here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * [Sync][C] remove dead examples/retool toctree refs examples/retool does not exist in vime (same dead-link situation as search-r1); drop it from the docs toctree in index.rst (en+zh). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * [Sync][C] docs: add Ray Distributed Debugger step-by-step section (port slime #2001) Appends the 'Step-by-Step Debugging with Ray Distributed Debugger' section (debugpy + RAY_DEBUG_POSTMORTEM + VS Code attach) to developer_guide/debug.md (en + zh). Content is engine-neutral (Ray/debugpy/VS Code), ported verbatim; no sglang->vllm translation needed. --------- Signed-off-by: aoshen02 <aoshen@inferact.ai> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Backports slime #1987 + #1990. Under --use-routing-replay (R3), the actor forward replays rollout routing while the reference forward uses normal routing, so their log-probs / KL are not expected to match bit-for-bit at rollout_id/step 0. The unconditional zero-KL CI asserts therefore raise false failures on any routing-replay + --ci-test run. - data.py: gate the rollout_id==0 log_probs vs ref_log_probs assert on (not use_rollout_routing_replay) and (not ci_disable_kl_checker). - model.py: gate the step-0 train/kl_loss assert on (not use_rollout_routing_replay). Signed-off-by: aoshen02 <aoshen@inferact.ai> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
What
Backports slime #1987 + #1990 (one logical fix, split across two upstream commits because #1990 amends #1987's lines and adds the matching
model.pyguard).Under
--use-routing-replay(R3), the actor forward replays rollout routing while the reference forward intentionally falls through to normal routing. Their log-probs / KL are therefore not expected to match bit-for-bit at the initial step. But our CI asserts assume exact zero-KL:data.py: atrollout_id == 0,assert abs(log_probs - ref_log_probs) < 1e-8model.py: at step 0,assert train/kl_loss < 1e-8So any routing-replay +
--ci-testrun raises a false failure. vime exercises this path today (e.g. the Qwen3-30B-A3B test passes--use-routing-replay).Fix
data.py: gate therollout_id == 0log_probs-vs-ref_log_probs assert onnot use_rollout_routing_replayandnot ci_disable_kl_checker(both viagetattrdefault-False for safety).model.py: gate the step-0train/kl_lossassert onnot use_rollout_routing_replay.Non-R3 runs are unaffected (the asserts still fire exactly as before).
Decoupling
Independent of #92 (GLM bridge) and the upcoming
logprob_abs_diffassert (slime #1968) / critic-temperature (slime #1928) PRs — no shared lines.Test
ast.parseclean on both files. Logic change is guard-only; non-R3 CI behavior identical.🤖 Generated with Claude Code