docs(training): align cookbook docs and skill references#421
Closed
Hecate0821 wants to merge 15 commits into
Closed
docs(training): align cookbook docs and skill references#421Hecate0821 wants to merge 15 commits into
Hecate0821 wants to merge 15 commits into
Conversation
Rebased onto main: this is a clean reapply of the renderer-backed RL rollout work (the original plan AC-1..13) on top of latest main. All checkpoint / GLM5 / weight_sync drift accumulated during the cancelled RLCR review loop has been dropped — those concerns are addressed (or intentionally out of scope) on main. What's in (the original 13 ACs): - AC-1 single_turn_renderer_rollout (utils/rl/renderer_rollout.py) — renderer-built prompts + sampler-returned tokens end-to-end via sample_with_prompt_tokens; multimodal raises MultimodalRenderingNotSupported - AC-2 multi_turn_minimal_renderer/rollout.py (concrete example using TrajectoryAssembler.add_call) - AC-3 inline 3-line extension-property guard inside the shared _renderer_turn_loop helper (NOT exported as a typed error) - AC-4 multi_turn_tool/rollout.py (renderer parses tool_calls; user-supplied env's execute runs tools; loss_mask=1 only on assistant tokens) - AC-5 RolloutService-backed remote-rollout helper (utils/rl/ text_rollout.py + rollout_service.py); RolloutPayload / TurnRecord schema unchanged - AC-6 ep_remote_grader/ refactored to renderer-built tokens (no chr(...) synthesis); not training-grade and documented as such - AC-7 NO parse/truncation policy enums, _apply_parse_policy helper, or parse_failure_total metric exported - AC-8 Renderer injection is explicit constructor arg; RolloutContext unchanged at the trainer surface - AC-9 The SDK side (sample_with_prompt_tokens) is in the SDK repo, outside this PR - AC-10 Parametrized RL-behavior tests for gemma4 + kimi_k25 - AC-11 Populated examples: single_turn_sync_on_policy/, single_turn_async/, multi_turn_minimal_renderer/, multi_turn_tool/, remote_rollout/, ep_remote_grader/, gsm8k_async/; updated multihop_qa migration; frozen_lake migration - AC-12 Trainer code zero parse/truncation policy enums, zero _apply_parse_policy, zero parse_failure_total, zero cookbook-local sampler shim - AC-13 Async RL loop (recipes/async_rl_loop.py) with per-turn output_versions plumbing Recipe-side hooks (minimal): - rl_loop.main(rollout_fn=..., ctx_extras=...) accepts a pluggable rollout function that receives RolloutContext, mirroring async_rl_loop.RolloutContext. The default sample_one_prompt path is preserved when rollout_fn=None. - utils/rl/losses.PromptGroup.prompt_lens (Optional[List[int]]) — per-sample prompt-token length for heterogeneous multi-turn rollouts (different prefix lengths per sample). None keeps the legacy single-prompt behavior. - utils/rl/metrics.compute_step_metrics(fwd_bwd_weights=...) — sample-weighted mean across PPO inner minibatches when the last minibatch is smaller than the rest (matches per-sample prompt_lens treatment in entropy). - utils/data.replicate_rows_for_epochs(rows, epochs) — independent deep-copied row dicts per epoch so rollout functions that mutate rows in place don't leak state across epochs. What's deliberately NOT in: - All checkpoint/resume changes (TrainingCheckpoints API, dataloader.json, legacy mirroring, _ResumeAnchor, record_cursor_only, durable counters, _durable_delta fallback). Those were either out of scope or accumulated drift from the cancelled review loop; correctness on main is unchanged by this PR. - All renderer (glm5.py) changes. Take main's version. - All SFT preserve-thinking changes (already on main). - DPO / IGPO / ORPO recipe changes (drift). - promote_checkpoint.py changes (drift). - weight_sync inline-in-train_step (doc-vs-impl semantics, no real correctness benefit; reverted in this rebase). Sanity checks: - All rollout-side imports resolve - training/tests/unit/test_rollout.py + test_renderer_rollout.py + test_rollout_helpers.py + test_rl_renderer_behavior.py + test_data_utils.py + tests/structural/: 77 pass / 18 skipped - training/tests/unit/test_rl_loop.py + test_checkpoints.py: 36 pass (no regression on main's existing tests) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ests Restructure the async RL recipe surface and remove redundant policy-as-test infrastructure that grew out of the original RLCR loop. - Move training/recipes/async_rl_loop.py -> training/advanced/async_rl.py; recipes/async_rl_loop.py is now a thin re-export shim so existing examples (single_turn_async, gsm8k_async, multi_turn_minimal, remote_rollout, ep_remote_grader) keep importing the legacy path. - Add empty training/examples/advanced/ scaffold for examples that exercise training.advanced.async_rl. - Drop renderer-heavy reference docs that were artifacts of the original audit: training/utils/rl/RENDERER_AUDIT.md and training/utils/rl/SCHEMA_DECISION.md. - Drop training/tests/structural/test_rl_helper_boundaries.py and training/tests/unit/test_rl_renderer_behavior.py as redundant policy-as-test (the boundaries are now reviewer-enforced and listed in training/utils/rl/README.md). - Drop TestAsyncMainReturnContract from test_async_rl_train.py (source-string inspection of main() rather than behavior). - Add fixture-only docstring to multi_turn_minimal_renderer/__init__.py pointing at runnable siblings (single_turn_async, ep_remote_grader). - Fix stale assembler.to_payload(...) -> pack_payload_to_sample chain in multi_turn_minimal_renderer/rollout.py and multi_turn_tool/rollout.py module docstrings (the code calls pack_assembled_to_sample). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reorganize rollout helpers into training/utils/rl/rollout/ as five focused modules (types, service, remote, renderer, assembler). Drop silent fallbacks introduced earlier in this branch: - rl_loop.main(rollout_fn=...) is required; the recipe ships a module-level default_rollout_fn instead of an in-main closure. - default_rollout_fn fails loud on sampler exceptions and on logprob/token misalignment (no try/except, no padding/truncation). - _dump_trajectory reads prompt/completions from row_meta only (no PromptGroup-field fallback). - Restore router_replay end-to-end through Rollout: RolloutSample carries routing_matrices; rollout_to_prompt_group aligns them via build_r3_routing_matrices and threads them into ModelInput.from_ints. Naming + merges: - text_rollout.py -> rollout/remote.py (the file was never about text) - make_text_rollout_fn -> make_remote_rollout_fn (drop alias) - rollout_helpers.py merged into rollout/assembler.py (the helpers build InferenceCall, which only the assembler consumes) - Drop unused RolloutHelperInfo / renderer_helper_info / helper_info triage scaffolding (only ever read by tests, no production caller) async_rl recipe stays at training/recipes/async_rl_loop.py (no shim). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Delete three AST-walk-the-sibling-train.py "tests" that only assert the example imports a specific symbol. They guard prior-review wiring choices, not behavior — an example that imports the right symbol but mis-wires it still passes. Smoke tests in training/tests/smoke_test/ already cover real breakage. Removed: - training/examples/rl/deepmath/test_deepmath_imports.py - training/examples/rl/gsm8k_async/test_gsm8k_async_imports.py - training/examples/rl/single_turn_sync_on_policy/test_train_imports.py Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the lazy-init dict-of-strings cache with eager construction in the closure. Tokenizer + renderer depend only on cfg, so build them once at the top of _build_rollout_fn; sampler depends on ctx and stays inline in rollout_fn (it's just a thin dataclass). Drops the per-row "if 'renderer' not in cached:" guard and the unused typing.Any imports. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous cleanup moved sampler construction into rollout_fn, which made it run per row. DeploymentSampler.__init__ chains into _RestClient which allocates an httpx.Client connection pool — the class has close() + __enter__/__exit__, so it owns resources. Per-row construction without close() leaks connection pools until GC. ctx.inference_base_url / model / api_key are populated once per training run (rl_loop.py:623) and never change, so per-row reconstruction was purely waste. Build once on the first rollout via a nonlocal sentinel, reuse on every subsequent row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The default zero-variance filter was named should_accept but is passed to dynamic_filter_fn=...; the call sites read more naturally with matching names. should_accept also reads as a question (boolean predicate), which is fine, but the symmetry with dynamic_filter_fn matters more for consumers who customize the filter. Renames the function in both rl_loop and igpo_loop recipes, all six example train.py files that override it, the two manual test scripts that monkey-patch it, and the unit/smoke tests that reference it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Make the boolean predicate semantics explicit: dynamic_filter_accept returns True when the group should be accepted (i.e. NOT filtered). The "_accept" suffix removes ambiguity about polarity for readers overriding the filter. Preserves the dynamic_filter_fn parameter name on main() / run_rl_loop; only the function symbol is renamed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…main Step back from the in-PR rl_loop refactor: rl_loop.py and igpo_loop.py go back to the origin/main versions (no RolloutContext, no default_rollout_fn, rollout_fn optional, should_accept name kept). Pre-existing examples, manual tests, and smoke/e2e tests that the PR had migrated to the new rl_loop API are likewise reset to main so the rl_loop ecosystem is identical to main. The PR-new sync example single_turn_sync_on_policy/ was the only non-test consumer of rl_loop's PR-only RolloutContext; it's removed so the new async loop stays fully separate from rl_loop. The renderer-reuse helpers under training/utils/rl/rollout/ and the new async_rl_loop.py + its async examples are untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The masking helpers live under training.examples.rl.frozen_lake, not training.examples.frozen_lake — importing the latter raised ModuleNotFoundError at module load and made train_multihop_qa_igpo.py unrunnable (and broke the smoke-imports test). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The renderer-reuse refactor at the head of this PR accidentally dropped render_messages_to_datums from training/utils/__init__.py while renaming neighboring symbols. sft_loop.py imports it from training.utils, so the smoke-imports test broke. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Eight small fixes uncovered by running the renderer-backed async loop and the
sync GRPO loop against a real 397B B300 deployment for the first time:
- async_rl_loop.py: drop obsolete kwargs that no longer exist on setup_infra
/ save_checkpoint surfaces (has_separate_lora_reference, warm_start_from_*,
step). The post-resolve checks live inside setup_infra now.
- logging.py: slime-aligned wandb step axes — train/step, rollout/step,
eval/step — so async-side perf/* and rollout/* don't pile on the train/step
axis when sampling and training are decoupled.
- async_train.py + metrics.py: rename async/step_wall_time -> step_wall_time
to match the metrics-side lookup key, and make the lookup defensive so the
async path (which writes the key after train_step) doesn't KeyError.
- common.py + rollout/types.py: switch per-token mask key from "loss_mask"
to "weights". The trainer SDK rejects any loss_fn_inputs key other than
{"target_tokens", "weights"} for forward_backward_custom.
- rollout/renderer.py: decode integer stop token IDs to literal strings via
ctx.tokenizer. The Fireworks completions API rejects int stops; Qwen's
renderer returns [151645] from get_stop_sequences().
- validation.py: add require_dataset=False so async recipes that pass rows=
directly (no JSONL path) can still run base_model / output_model_id checks.
Plus dataset.jsonl regenerated to 500 rows from zwhe99/DeepMath-103K (was 200).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- recipes.md: rename IGPO row from "Importance-weighted GRPO" to "Information Gain-based Policy Optimization (IGPO)" — matches the recipe's actual algorithm (Wang et al., ICLR 2026). - examples.md: add multi_turn_minimal, gsm8k_async, ep_remote_grader; drop the misleading "Generic RL wiring → examples/rl/" parent-dir row; relabel Multi-hop QA RL → Multi-hop QA IGPO. - training/README.md: add Async RL and IGPO recipe rows; expand the Directory layout block to cover every rl/ example subdir + multihop_qa. - checkpoints.md: include async_rl_loop in the warm_start support list (the recipe already wires validate_warm_start_config). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hecate0821
force-pushed
the
chengxi/training-doc-skill-alignment
branch
from
May 4, 2026 22:36
6865e57 to
ea5c39c
Compare
Collaborator
Author
|
Closing in favor of a smaller, doc-only PR. The renderer-backed RL refactor / async-loop work in this branch is superseded by what's already on main (#382 etc.); the audit-fix docs are split out so they can land independently. |
4 tasks
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.
Summary
Verification
Not run