Repository navigation
refactor(algo)!: sft's teacher replaces algo.sampling.source; opsd uses the policy renderer - #3987
Open
faresobeid wants to merge 11 commits into
Open
faresobeid wants to merge 11 commits into
faresobeid wants to merge 11 commits into
Conversation
faresobeid
marked this pull request as ready for review
October 11, 2026 12:45
…laces-sampling-source # Conflicts: # packages/prime-rl-configs/src/prime_rl/configs/algorithm.py
# Conflicts: # packages/prime-rl-configs/src/prime_rl/configs/orchestrator.py
…laces-sampling-source # Conflicts: # packages/prime-rl-configs/src/prime_rl/configs/algorithm.py
…laces-sampling-source
# Conflicts: # packages/prime-rl-configs/src/prime_rl/configs/algorithm.py # packages/prime-rl-configs/src/prime_rl/configs/orchestrator.py # packages/prime-rl-configs/src/prime_rl/configs/rl.py # src/prime_rl/orchestrator/envs.py
…laces-sampling-source
This branch has not been deployed
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
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.
sft's teacher replaces
algo.sampling.source, and opsd renders hints with the policy's renderer.Changes
algo.samplingis removed (SamplingConfig,BaseAlgoConfig.samplingand its source/loss validator).sampling.sourcecould only be"policy"for 8 of the 9 algorithms and was only meaningful forsft. Its name also collided withtrain.sampling(temperature, top_p, …) on the same parent.SFTAlgoConfig.teacher: FrozenModelConfigis required, the same type asopd.teacher, so both distillation teachers are configured as[orchestrator.train.algo.teacher].algo.rollout_modelreturns"policy", or the teacher for sft. It replaces everyalgo.sampling.sourceread in the orchestrator, the config validators (including the replayprl_replaysetup) and env setup. The rl entrypoint lists the frozen teacher endpoints of opd and sft once."policy"cannot be a teacher, so "sft needs a frozen source" and "rl/ref_kl must sample from the policy" are now enforced by the types instead of validators.opsd.rendereris removed. Its docstring asked users to set it by hand to match the policy renderer. Hints are now rendered with the policy client's renderer config (orchestrator.renderer) and the sameload_tokenizer(model.name)tokenizer as before.Breaking:
[orchestrator.train.algo.sampling.source]→[orchestrator.train.algo.teacher];opsd.rendereris gone. The three shipped sft configs,docs/algorithms.md,docs/training.md,configs/debug/algo/README.mdandskills/configs/SKILL.mdare updated.Validation
pytest tests/unit/orchestrator tests/unit/test_configs.py tests/unit/inference/test_dynamo.py -m "not gpu": 401 passed.test_load_configsloads every TOML inconfigs/andexamples/, so the sft configs are covered.InferenceClientexposestrain_client.renderer(the config opsd now reads).ruff check,ruff format --check: clean. No dependency changes.Overlaps
Open PRs #3699 (PPO/SAO), #3685 and #3640 (CriPO-S) build on
BaseAlgoConfig. After this lands they should dropsampling(and the CriPO-S per-algorithmrenderer) and userollout_modeland the policy renderer.Note
Medium Risk
Breaking TOML/API change for SFT configs and OPSD renderer overrides; behavior is intended to stay equivalent but mis-migrated configs will fail at validation or change hint tokenization if users relied on a custom opsd renderer.
Overview
Breaking config refactor for how algorithms declare which model generates train rollouts and how SFT/OPSD wire frozen models and renderers.
algo.samplingis removed (SamplingConfig, nestedsampling.source, and cross-algorithm validators). Rollout sourcing is expressed via arollout_modelproperty on each algo config:"policy"for all algorithms exceptsft, which returns its requiredteacher. Orchestrator validators, env setup, sampling replay (prl_replay), and the RL entrypoint’s frozen-endpoint warnings now key offrollout_modelinstead ofsampling.source.SFT now requires
[orchestrator.train.algo.teacher]with the sameFrozenModelConfigtype as OPD (replacing[orchestrator.train.algo.sampling.source]). Type requirements replace runtime checks that forbade policy-as-teacher or frozen sampling for RL/ref_kl algos.OPSD drops
opsd.renderer; demonstration hints use the policy inference client’s renderer config (orchestrator.renderer) and tokenizer insetup().Shipped SFT TOMLs, algorithm/training docs, debug README, and config skill text are updated to the new teacher path.
Reviewed by Cursor Bugbot for commit b7256ff. Bugbot is set up for automated code reviews on this repo. Configure here.