From 07e94cebbce896c154293322daea59ab4a7aa37c Mon Sep 17 00:00:00 2001 From: Alpamys Date: Mon, 6 Jul 2026 15:27:11 +0500 Subject: [PATCH] fix(v0.71.31): trl-version adapter for online_dpo (CI red on trl 1.x) CI resolves trl 1.7.1 (local dev is 0.19.1); trl 1.x removed pairwise judges (BasePairwiseJudge) and moved OnlineDPOTrainer to trl.experimental.online_dpo with a reward_funcs= API. Add a runtime adapter: - _import_online_dpo() (stable 0.19.x -> experimental 1.x fallback) - _trl_has_judges() capability check drives the branch - judge leg: judge=make_soup_pairwise_judge (pairwise, 0.19.x) OR reward_funcs=[make_judge_reward_func] (pointwise, 1.x) via the SAME httpx JudgeEvaluator best-of-N uses - reward_model leg: reward_model=/reward_processing_class= (0.19.x) OR reward_funcs=[rm]/reward_processing_classes= (1.x) - test seam is now a Soup evaluator (compare_pair + evaluate), adapted per version Tests version-adaptive: TestSoupPairwiseJudge skips on trl>=1.0; new TestJudgeRewardFunc covers the 1.x adapter always; build/kwargs tests branch on trl version. Validated: full test_v07131 on trl 0.19.1 (84 passed); adaptation logic + OnlineDPO import + trainer build on a clean trl 1.7.1 / torch 2.6 venv (reward_funcs path reached model load; per-version pairwise-vs-pointwise difference documented). --- CHANGELOG.md | 5 +- src/soup_cli/eval/judge.py | 71 +++++++++++++++ src/soup_cli/trainer/online_dpo.py | 101 ++++++++++++++++----- tests/test_v07131.py | 138 ++++++++++++++++++++++------- 4 files changed, 259 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b664533..b7cf1bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,10 @@ reproducing 70+ versions of notes. `training.online_dpo_judge: "ollama://model"` (or set `reward_model` — exactly one), `online_dpo_loss_type: sigmoid|ipo`, `online_dpo_max_new_tokens`; `beta` reuses `dpo_beta`. Transformers + text only. Recipe: - `online-dpo-smollm2-135m`. + `online-dpo-smollm2-135m`. Adapts to the installed TRL: on trl 0.19.x the + judge is a swap-debiased *pairwise* comparison; on trl 1.x (which removed + pairwise judges) the same `JudgeEvaluator` is used as a *pointwise* + reward function — a documented per-version behaviour difference. - **`soup data best-of-n`** — Best-of-N rejection sampling (BOND-lite): sample N completions from `--base` locally, a `--judge` scores each pointwise, and the winner is written as an SFT chat row (with provenance). `--emit-pairs` diff --git a/src/soup_cli/eval/judge.py b/src/soup_cli/eval/judge.py index 41d997a..e799c06 100644 --- a/src/soup_cli/eval/judge.py +++ b/src/soup_cli/eval/judge.py @@ -441,6 +441,77 @@ def pairwise_winrate( return total / len(pairs) +def _as_prompt_text(prompt) -> str: + """Best-effort text of a prompt (string or conversational message list).""" + if isinstance(prompt, str): + return prompt + if isinstance(prompt, list): + users = [ + m["content"] + for m in prompt + if isinstance(m, dict) and m.get("role") == "user" + and isinstance(m.get("content"), str) + ] + if users: + return users[-1] + return " ".join( + m["content"] + for m in prompt + if isinstance(m, dict) and isinstance(m.get("content"), str) + ) + return str(prompt) + + +def _as_completion_text(completion) -> str: + """Best-effort text of a completion (string, message, or message list).""" + if isinstance(completion, str): + return completion + if isinstance(completion, dict): + content = completion.get("content") + return content if isinstance(content, str) else str(completion) + if isinstance(completion, list): + return " ".join( + m["content"] + for m in completion + if isinstance(m, dict) and isinstance(m.get("content"), str) + ) + return str(completion) + + +def make_judge_reward_func(evaluator: object, *, name: str = "soup_judge"): + """Build a trl-1.x OnlineDPO POINTWISE reward function from a Soup evaluator. + + trl 1.x removed the pairwise-judge Online DPO API (``BasePairwiseJudge``) and + ranks the two on-policy completions by a per-completion ``reward_funcs`` + signal instead. This adapts Soup's ``JudgeEvaluator`` by scoring each + completion pointwise with ``evaluator.evaluate(prompt, completion)`` — the + SAME pointwise judge ``soup data best-of-n`` uses — returning its + ``weighted_score``. Note the semantic difference vs the trl-0.19.x path, + which uses the swap-debiased *pairwise* comparison; per-version behaviour is + documented as a known difference. + + The returned callable matches trl's reward-func contract + ``fn(prompts, completions, **kwargs) -> list[float]`` and carries ``name`` as + ``__name__`` so trl logs ``rewards/``. + """ + + def _reward(prompts, completions, **kwargs): + scores: list[float] = [] + for prompt, completion in zip(prompts, completions, strict=False): + prompt_text = _as_prompt_text(prompt) + completion_text = _as_completion_text(completion) + try: + score = evaluator.evaluate(prompt_text, completion_text).weighted_score + scores.append(float(score)) + except Exception as exc: # noqa: BLE001 — judge/network variety + logger.debug("judge reward func failed: %s", exc) + scores.append(0.0) + return scores + + _reward.__name__ = name + return _reward + + def _base_pairwise_judge_cls() -> type: """Lazily import TRL's ``BasePairwiseJudge`` with a friendly error.""" try: diff --git a/src/soup_cli/trainer/online_dpo.py b/src/soup_cli/trainer/online_dpo.py index 4906595..593db7b 100644 --- a/src/soup_cli/trainer/online_dpo.py +++ b/src/soup_cli/trainer/online_dpo.py @@ -2,16 +2,28 @@ Unlike offline DPO (which reads static ``prompt/chosen/rejected`` rows), Online DPO generates two completions per prompt ON-POLICY at each step and asks a -*judge* (a pairwise LLM judge) OR a *reward model* which is better — the winner -becomes ``chosen``, the loser ``rejected``. The judge closes the loop. +*judge* (an LLM judge) OR a *reward model* which is better — the winner becomes +``chosen``, the loser ``rejected``. The judge closes the loop. Data is prompt-only (like GRPO): Soup's ``{"messages": [...]}`` rows are normalized to the OnlineDPO ``prompt`` column (chat, minus the assistant turn). -The judge is a Soup :func:`soup_cli.eval.judge.make_soup_pairwise_judge` adapter -over the project's httpx ``JudgeEvaluator`` (ollama / server / openai), so it -works with a local judge on the dev box. ``_ONLINE_DPO_JUDGE_OVERRIDE`` is a -test seam for injecting a synthetic judge (offline proof-of-mechanism). +**Cross-version adapter.** TRL changed the OnlineDPO API between 0.19.x and 1.x: + +- **trl 0.19.x** — ``from trl import OnlineDPOTrainer``; the judge is a + ``BasePairwiseJudge`` (swap-debiased *pairwise* comparison, via + :func:`soup_cli.eval.judge.make_soup_pairwise_judge`); a reward model is + passed as ``reward_model=`` / ``reward_processing_class=``. +- **trl 1.x** — pairwise judges were removed; ``OnlineDPOTrainer`` moved to + ``trl.experimental.online_dpo`` and ranks completions by ``reward_funcs=``. The + same Soup ``JudgeEvaluator`` is adapted to a *pointwise* reward function + (:func:`soup_cli.eval.judge.make_judge_reward_func`) — the same pointwise judge + ``soup data best-of-n`` uses. Reward models pass as ``reward_funcs=[rm]`` / + ``reward_processing_classes=[tok]``. + +``_ONLINE_DPO_JUDGE_OVERRIDE`` is a test seam for injecting a synthetic Soup +evaluator (has ``.compare_pair`` + ``.evaluate``); it is adapted to whichever +API the installed trl exposes. """ import time @@ -26,9 +38,42 @@ from soup_cli.utils.gpu import estimate_batch_size, model_size_from_name console = Console() # Test seam: when set, replaces the URL-built judge (used by the offline -# synthetic-judge smoke). A ``trl.BasePairwiseJudge`` instance or None. +# synthetic-judge smoke). A Soup evaluator (``.compare_pair`` + ``.evaluate``). _ONLINE_DPO_JUDGE_OVERRIDE = None + +def _import_online_dpo(): + """Import ``OnlineDPOConfig``/``OnlineDPOTrainer`` across trl versions. + + trl 0.19.x exposes them at the top level; trl 1.x moved them to + ``trl.experimental.online_dpo``. + """ + try: + from trl import OnlineDPOConfig, OnlineDPOTrainer + + return OnlineDPOConfig, OnlineDPOTrainer + except ImportError: + pass + try: + from trl.experimental.online_dpo import OnlineDPOConfig, OnlineDPOTrainer + + return OnlineDPOConfig, OnlineDPOTrainer + except ImportError as exc: # pragma: no cover — trl ships in [train] + raise ImportError( + "task='online_dpo' requires trl with OnlineDPO support " + "(pip install 'soup-cli[train]')" + ) from exc + + +def _trl_has_judges() -> bool: + """True on trl 0.19.x (pairwise ``BasePairwiseJudge`` API), False on trl 1.x.""" + try: + from trl import BasePairwiseJudge # noqa: F401 + + return True + except ImportError: + return False + # Fallback chat template for base models that ship none. Unlike the shared # ``constants.DEFAULT_CHAT_TEMPLATE``, this one emits an assistant generation # cue on ``add_generation_prompt`` so on-policy generation continues the @@ -115,13 +160,7 @@ class OnlineDPOTrainerWrapper: """Load model, tokenizer, build the OnlineDPO trainer (judge in loop).""" from datasets import Dataset - try: - from trl import OnlineDPOConfig, OnlineDPOTrainer - except ImportError as exc: # pragma: no cover — trl ships in [train] - raise ImportError( - "task='online_dpo' requires trl>=0.19 with OnlineDPO support " - "(pip install 'soup-cli[train]')" - ) from exc + OnlineDPOConfig, OnlineDPOTrainer = _import_online_dpo() # noqa: N806 (classes) from soup_cli.trainer.sft import _enable_hf_transfer_progress @@ -288,27 +327,37 @@ class OnlineDPOTrainerWrapper: apply_pre_lora_patches(self.model, cfg.base) + @staticmethod + def _judge_kwargs(evaluator, has_judges: bool) -> dict: + """Adapt a Soup evaluator to the installed trl's judge/reward API.""" + if has_judges: # trl 0.19.x — swap-debiased pairwise judge + from soup_cli.eval.judge import make_soup_pairwise_judge + + return {"judge": make_soup_pairwise_judge(evaluator)} + # trl 1.x — pointwise reward function (judges were removed) + from soup_cli.eval.judge import make_judge_reward_func + + return {"reward_funcs": [make_judge_reward_func(evaluator)]} + def _build_judge_or_reward(self, tcfg) -> dict: """Resolve the OnlineDPO reward signal: judge OR reward_model. Precedence: the test seam, then the judge URL, then a reward model. The schema cross-validator guarantees exactly one of judge/reward is set for - a real config. + a real config. The returned kwargs adapt to the installed trl version + (``judge=`` on 0.19.x, ``reward_funcs=`` on 1.x). """ + has_judges = _trl_has_judges() if _ONLINE_DPO_JUDGE_OVERRIDE is not None: - return {"judge": _ONLINE_DPO_JUDGE_OVERRIDE} + return self._judge_kwargs(_ONLINE_DPO_JUDGE_OVERRIDE, has_judges) if tcfg.online_dpo_judge: from soup_cli.eval.gate import _parse_judge_url - from soup_cli.eval.judge import ( - JudgeEvaluator, - make_soup_pairwise_judge, - validate_judge_api_base, - ) + from soup_cli.eval.judge import JudgeEvaluator, validate_judge_api_base provider, model, api_base = _parse_judge_url(tcfg.online_dpo_judge) validate_judge_api_base(api_base) evaluator = JudgeEvaluator(provider=provider, model=model, api_base=api_base) - return {"judge": make_soup_pairwise_judge(evaluator)} + return self._judge_kwargs(evaluator, has_judges) if tcfg.reward_model: from transformers import ( AutoModelForSequenceClassification, @@ -323,7 +372,13 @@ class OnlineDPOTrainerWrapper: reward_tok = AutoTokenizer.from_pretrained( tcfg.reward_model, trust_remote_code=self._trust_remote_code ) - return {"reward_model": reward, "reward_processing_class": reward_tok} + if has_judges: # trl 0.19.x + return {"reward_model": reward, "reward_processing_class": reward_tok} + # trl 1.x — a reward model is one of reward_funcs + return { + "reward_funcs": [reward], + "reward_processing_classes": [reward_tok], + } raise ValueError( "online_dpo needs training.online_dpo_judge or training.reward_model" ) diff --git a/tests/test_v07131.py b/tests/test_v07131.py index 5f636f7..d226ac1 100644 --- a/tests/test_v07131.py +++ b/tests/test_v07131.py @@ -8,13 +8,32 @@ pairwise`` (#284), ``task='online_dpo'`` (schema + trainer + routing), from __future__ import annotations +import pytest + # --------------------------------------------------------------------------- # Shared test doubles # --------------------------------------------------------------------------- +def _trl_has_judges(): + """True on trl 0.19.x (pairwise BasePairwiseJudge API), False on trl 1.x.""" + try: + from trl import BasePairwiseJudge # noqa: F401 + + return True + except ImportError: + return False + + +_TRL_HAS_JUDGES = _trl_has_judges() + + class _FakeJudge: - """Deterministic pairwise judge: prefers the LONGER response (A vs B).""" + """Synthetic length-preferring Soup evaluator: prefers the LONGER response. + + Provides BOTH shapes so it drives either trl API: ``compare_pair`` (pairwise, + trl 0.19.x) and ``evaluate`` (pointwise, trl 1.x reward-func path). + """ def __init__(self, rubric=None): self.rubric = rubric or {"scale": {"min": 1, "max": 5}, "criteria": []} @@ -24,6 +43,13 @@ class _FakeJudge: return -1 return 0 if len(resp_a) > len(resp_b) else 1 + def evaluate(self, prompt, response, category="default"): + from soup_cli.eval.judge import JudgeScore + + return JudgeScore( + prompt=prompt, response=response, weighted_score=float(len(response)) + ) + class _PosBias: """A biased judge that ALWAYS says the first response is best.""" @@ -152,10 +178,16 @@ class TestCompareePairMethod: # --------------------------------------------------------------------------- -# Task 2 — make_soup_pairwise_judge (TRL BasePairwiseJudge adapter) +# Task 2 — make_soup_pairwise_judge (trl 0.19.x pairwise BasePairwiseJudge +# adapter). trl 1.x removed judges -> the 1.x-equivalent coverage is +# TestJudgeRewardFunc below (the pointwise reward-func adapter). Together they +# cover whichever adapter the installed trl actually exposes. # --------------------------------------------------------------------------- +@pytest.mark.skipif( + not _TRL_HAS_JUDGES, reason="pairwise BasePairwiseJudge adapter is trl<1.0 only" +) class TestSoupPairwiseJudge: def test_judge_returns_best_index(self): from soup_cli.eval.judge import make_soup_pairwise_judge @@ -191,6 +223,45 @@ class TestSoupPairwiseJudge: assert isinstance(make_soup_pairwise_judge(_FakeJudge()), BasePairwiseJudge) +# --------------------------------------------------------------------------- +# make_judge_reward_func (trl 1.x pointwise reward-func adapter) — version- +# independent (uses evaluator.evaluate), so it runs on every trl. +# --------------------------------------------------------------------------- + + +class TestJudgeRewardFunc: + def test_named_and_scores_by_evaluate(self): + from soup_cli.eval.judge import make_judge_reward_func + + fn = make_judge_reward_func(_FakeJudge()) + assert fn.__name__ == "soup_judge" + # _FakeJudge.evaluate -> weighted_score == len(response) + scores = fn(["p", "p"], ["ab", "abcd"]) + assert scores == [2.0, 4.0] + + def test_custom_name(self): + from soup_cli.eval.judge import make_judge_reward_func + + assert make_judge_reward_func(_FakeJudge(), name="mine").__name__ == "mine" + + def test_conversational_prompt_and_completion(self): + from soup_cli.eval.judge import make_judge_reward_func + + fn = make_judge_reward_func(_FakeJudge()) + prompts = [[{"role": "user", "content": "hi"}]] + completions = [[{"role": "assistant", "content": "abcd"}]] + assert fn(prompts, completions) == [4.0] + + def test_evaluate_failure_scores_zero(self): + from soup_cli.eval.judge import make_judge_reward_func + + class _Boom: + def evaluate(self, prompt, response, category="default"): + raise RuntimeError("network down") + + assert make_judge_reward_func(_Boom())(["p"], ["x"]) == [0.0] + + # --------------------------------------------------------------------------- # Task 3 — soup ship --task-mode pairwise (#284) # --------------------------------------------------------------------------- @@ -430,17 +501,6 @@ class TestOnlineDpoSchema: # --------------------------------------------------------------------------- -def _make_len_judge(): - """A synthetic length-preferring BasePairwiseJudge (no network).""" - from trl import BasePairwiseJudge - - class _LenJudge(BasePairwiseJudge): - def judge(self, prompts, completions, shuffle_order=True): - return [0 if len(c[0]) >= len(c[1]) else 1 for c in completions] - - return _LenJudge() - - class TestOnlineDpoWrapper: def test_prompt_rows_from_messages(self): from soup_cli.trainer.online_dpo import OnlineDPOTrainerWrapper @@ -514,13 +574,18 @@ class TestOnlineDpoWrapper: ) assert out == [] - def test_synthetic_judge_prefers_longer(self): - j = _make_len_judge() - # completion 0 longer -> index 0; completion 1 longer -> index 1 - assert j.judge(["p"], [["longer", "s"]]) == [0] - assert j.judge(["p"], [["s", "longer"]]) == [1] + def test_synthetic_evaluator_double_both_shapes(self): + # The seam is a Soup evaluator (compare_pair + evaluate), adapted per + # trl version. Verify both shapes on the length-preferring double. + ev = _FakeJudge() + assert ev.compare_pair("p", "longer", "s") == 0 + assert ev.compare_pair("p", "s", "longer") == 1 + assert ev.evaluate("p", "abcd").weighted_score == 4.0 def test_setup_builds_trainer_with_synthetic_judge(self): + # Parametrized over trl version by the wrapper itself: the seam evaluator + # is adapted to judge= (trl 0.19.x) or reward_funcs= (trl 1.x). Builds a + # real trainer on either. import soup_cli.trainer.online_dpo as od from soup_cli.config.loader import load_config_from_string from soup_cli.trainer.online_dpo import OnlineDPOTrainerWrapper @@ -531,7 +596,7 @@ class TestOnlineDpoWrapper: "training:\n online_dpo_judge: \"ollama://m\"\n" " epochs: 1\n batch_size: 2\n online_dpo_max_new_tokens: 8\n" ) - od._ONLINE_DPO_JUDGE_OVERRIDE = _make_len_judge() + od._ONLINE_DPO_JUDGE_OVERRIDE = _FakeJudge() try: wrapper = OnlineDPOTrainerWrapper(cfg, device="cpu") wrapper.setup( @@ -564,17 +629,21 @@ def _online_dpo_wrapper(): class TestBuildJudgeOrReward: - def test_judge_url_branch(self): - from trl import BasePairwiseJudge - + def test_judge_url_branch_adapts_to_trl_version(self): import soup_cli.trainer.online_dpo as od od._ONLINE_DPO_JUDGE_OVERRIDE = None result = _online_dpo_wrapper()._build_judge_or_reward(_Tcfg(judge="ollama://m")) - assert isinstance(result["judge"], BasePairwiseJudge) - assert "reward_model" not in result + if _TRL_HAS_JUDGES: # trl 0.19.x -> pairwise judge= + from trl import BasePairwiseJudge - def test_reward_model_branch(self, monkeypatch): + assert isinstance(result["judge"], BasePairwiseJudge) + assert "reward_funcs" not in result + else: # trl 1.x -> pointwise reward_funcs= + assert callable(result["reward_funcs"][0]) + assert "judge" not in result + + def test_reward_model_branch_adapts_to_trl_version(self, monkeypatch): import soup_cli.trainer.online_dpo as od od._ONLINE_DPO_JUDGE_OVERRIDE = None @@ -586,22 +655,27 @@ class TestBuildJudgeOrReward: "transformers.AutoTokenizer.from_pretrained", lambda *a, **k: object() ) result = _online_dpo_wrapper()._build_judge_or_reward(_Tcfg(reward="some/rm")) - assert set(result.keys()) == {"reward_model", "reward_processing_class"} + if _TRL_HAS_JUDGES: # trl 0.19.x + assert set(result.keys()) == {"reward_model", "reward_processing_class"} + else: # trl 1.x — a reward model is one of reward_funcs + assert set(result.keys()) == {"reward_funcs", "reward_processing_classes"} + assert len(result["reward_funcs"]) == 1 def test_precedence_override_wins(self): import soup_cli.trainer.online_dpo as od - sentinel = object() - od._ONLINE_DPO_JUDGE_OVERRIDE = sentinel + od._ONLINE_DPO_JUDGE_OVERRIDE = _FakeJudge() try: - result = _online_dpo_wrapper()._build_judge_or_reward(_Tcfg(judge="ollama://m")) - assert result["judge"] is sentinel # seam beats the URL branch + # A URL that _parse_judge_url would REJECT if the URL branch ran. + # No exception -> the seam took precedence (URL never parsed). + result = _online_dpo_wrapper()._build_judge_or_reward( + _Tcfg(judge="not-a-valid-url") + ) + assert "judge" in result or "reward_funcs" in result finally: od._ONLINE_DPO_JUDGE_OVERRIDE = None def test_neither_raises(self): - import pytest - import soup_cli.trainer.online_dpo as od od._ONLINE_DPO_JUDGE_OVERRIDE = None