diff --git a/tests/test_rl_positive_lm_success_check.py b/tests/test_rl_positive_lm_success_check.py new file mode 100644 index 00000000..ab1fafd0 --- /dev/null +++ b/tests/test_rl_positive_lm_success_check.py @@ -0,0 +1,88 @@ +"""positive_lm_success launch check + gsm8k success flag (S19 #6 s19-vapo-g3-20261010a). + +The G3 run loaded the out-of-tree ``gsm8k_reward:score`` (no success flag) with a +VAPO spec using ``positive_lm_source='success'``; the Miles fork raised on both +islands at the first critic step. S13 G1 passed with ``yeto.rl.gsm8k_reward:score``. +""" + +from __future__ import annotations + +import asyncio +import sys +import types + +from yeto.rl.algos.critic import positive_lm_success_problems +from yeto.rl.algos.vapo import vapo_spec +from yeto.rl.engine.algorithm import launch_problems +from yeto.rl.reward_declarations import writes_success + +_VALS = {"rollout_batch_size": 4, "rollout_max_response_len": 384, + "context_parallel_size": 1, "multi_lora": False} + + +def _spec(source="success"): + if source == "success": + return vapo_spec() + return vapo_spec(loss={"positive_lm_source": source, "positive_lm_reward_threshold": 0.0}) + + +def _install_undeclared(monkeypatch): + mod = types.ModuleType("_oot_gsm8k_reward") + + async def score(args, sample, **kw): # out-of-tree copy: reward only + return 1.0 + + mod.score = score + monkeypatch.setitem(sys.modules, "_oot_gsm8k_reward", mod) + + +def test_vapo_spec_uses_success_source(): + assert _spec().loss.positive_lm_source == "success" + + +def test_undeclared_reward_is_refused(monkeypatch): + _install_undeclared(monkeypatch) + vals = dict(_VALS, reward_function="_oot_gsm8k_reward:score") + probs = positive_lm_success_problems(_spec(), vals) + assert probs and "writes_success" in probs[0] + assert any(p.startswith("[positive_lm_success]") for p in launch_problems(_spec(), vals)) + + +def test_unimportable_reward_gets_no_verdict(): + assert positive_lm_success_problems(_spec(), dict(_VALS, reward_function="no_such_mod_xyz:score")) == [] + + +def test_user_reward_with_declaration_passes(monkeypatch): + mod = types.ModuleType("_user_reward") + + @writes_success + async def score(args, sample, **kw): + sample.metadata["success"] = True + return 1.0 + + mod.score = score + monkeypatch.setitem(sys.modules, "_user_reward", mod) + assert positive_lm_success_problems(_spec(), dict(_VALS, reward_function="_user_reward:score")) == [] + + +def test_declared_in_package_rewards_pass(): + for rf in ("yeto.rl.gsm8k_reward:score", "yeto.rl.math_reward:reward_func"): + assert positive_lm_success_problems(_spec(), dict(_VALS, reward_function=rf)) == [] + + +def test_skips_without_reward_or_other_source(monkeypatch): + _install_undeclared(monkeypatch) + assert positive_lm_success_problems(_spec(), _VALS) == [] + vals = dict(_VALS, reward_function="_oot_gsm8k_reward:score") + assert positive_lm_success_problems(_spec("reward"), vals) == [] + from yeto.rl.engine.algorithm import AlgorithmSpec + assert positive_lm_success_problems(AlgorithmSpec(), vals) == [] + + +def test_gsm8k_score_writes_success_matching_reward(): + from yeto.rl.gsm8k_reward import score + for response, want in (("so \\boxed{72}", 1.0), ("so \\boxed{71}", 0.0), ("", 0.0)): + sample = types.SimpleNamespace(response=response, label="... #### 72", metadata={}) + value = asyncio.run(score(None, sample)) + assert value == want + assert sample.metadata["success"] is (want == 1.0) diff --git a/yeto/launcher.py b/yeto/launcher.py index 8e83f4bf..00ca4369 100644 --- a/yeto/launcher.py +++ b/yeto/launcher.py @@ -1648,6 +1648,7 @@ def _prepare_ports_algorithm(args, rl_engine: str) -> None: # rl-algo-critic-family 2.3 (critic run-level rejections) "sync_preset": getattr(args, "rl_sync_preset", "strict-avg"), "elastic": bool(getattr(args, "rl_elastic", False)), + "reward_function": getattr(args, "reward_function", None), }) if problems: raise AlgorithmSpecError("algorithm spec rejected: " + "; ".join(problems)) diff --git a/yeto/rl/adapters/miles/config.py b/yeto/rl/adapters/miles/config.py index 1c7d6148..89ab3f8d 100644 --- a/yeto/rl/adapters/miles/config.py +++ b/yeto/rl/adapters/miles/config.py @@ -674,6 +674,7 @@ def translate_run_config( "multi_lora": any(t.split("=", 1)[0] == "--multi-lora" for t in extra_argv), # rl-algo-critic-family 2.3: critic GPU counts / deploy-component / indep-dp "extra_argv": tuple(extra_argv), + "reward_function": config.algorithm.reward_function, "actor_num_nodes": config.parallel.actor_num_nodes, "actor_num_gpus_per_node": config.parallel.actor_num_gpus_per_node, "elastic": bool(config.use_miles_router), # set iff --rl-elastic (run_config) diff --git a/yeto/rl/algos/critic.py b/yeto/rl/algos/critic.py index 8ed9cb26..32e3a365 100644 --- a/yeto/rl/algos/critic.py +++ b/yeto/rl/algos/critic.py @@ -99,6 +99,37 @@ def _reject_positive_lm(spec: AlgorithmSpec) -> str | None: register_rejection("positive_lm_threshold", _reject_positive_lm) + +def positive_lm_success_problems(spec: AlgorithmSpec, values: Mapping[str, Any]) -> list[str]: + """``loss.positive_lm_source='success'`` needs a reward function declared + (``yeto.rl.reward_declarations``) to write ``sample.metadata['success']``. + + Without it the Miles fork raises at the first critic step on the island + (S19 #6 s19-vapo-g3-20261010a: out-of-tree ``gsm8k_reward:score`` wrote no + flag; S13 G1 passed with ``yeto.rl.gsm8k_reward:score``). Skipped when the + caller passes no ``reward_function`` or it cannot be imported here. + """ + + if spec.loss.positive_lm_coef is None or spec.loss.positive_lm_source != "success": + return [] + reward_function = values.get("reward_function") + if not reward_function: + return [] + from yeto.rl.reward_declarations import reward_declares_success + + # None (not importable here): no verdict; the Miles fork still refuses on + # the island. Sub-agent decision: test fixtures use placeholder references. + if reward_declares_success(reward_function) is not False: + return [] + return [f"loss.positive_lm_source='success' needs a reward function that writes " + f"sample.metadata['success']; {reward_function} is not declared with " + "yeto.rl.reward_declarations.writes_success. Use e.g. " + "yeto.rl.gsm8k_reward:score, or positive_lm_source='reward' only when a " + "positive reward always means complete success"] + + +register_launch_check("positive_lm_success", positive_lm_success_problems) + # -------------------------------------------------------------------------- # Stage-W value-quality gate (user decision B, design D5/D7). Absent (None) = # record only, never block; absent from the canonical JSON while None, so no diff --git a/yeto/rl/reward_declarations.py b/yeto/rl/reward_declarations.py new file mode 100644 index 00000000..d40b15bb --- /dev/null +++ b/yeto/rl/reward_declarations.py @@ -0,0 +1,42 @@ +"""Declarations about what a reward function writes into ``sample.metadata``. + +``loss.positive_lm_source='success'`` (VAPO positive-example LM loss) needs the +reward function to write the boolean ``sample.metadata['success']``. The launch +check ``positive_lm_success`` (yeto.rl.algos.critic) asks this module. + +Kept apart from the reward modules on purpose: their source hashes are part of +the decoupling golden samples (tests/decoupling_golden.py). +""" + +from __future__ import annotations + +import importlib + +SUCCESS_DECLARATION_ATTR = "yeto_writes_success" + +# In-package Miles reward entry points that call yeto.rl.math_reward.set_success. +SUCCESS_WRITING_REWARDS = frozenset({ + "yeto.rl.gsm8k_reward:score", + "yeto.rl.math_reward:reward_func", +}) + + +def writes_success(fn): + """Declare that reward function ``fn`` writes ``sample.metadata['success']``.""" + + setattr(fn, SUCCESS_DECLARATION_ATTR, True) + return fn + + +def reward_declares_success(reward_function: str) -> bool | None: + """True: declared; False: importable but undeclared; None: cannot import here.""" + + ref = str(reward_function) + if ref in SUCCESS_WRITING_REWARDS: + return True + module, _, name = ref.partition(":") + try: + fn = getattr(importlib.import_module(module), name) + except Exception: # noqa: BLE001 - any import failure means "cannot confirm" + return None + return bool(getattr(fn, SUCCESS_DECLARATION_ATTR, False))