diff --git a/CHANGELOG.md b/CHANGELOG.md index c23cb98..10115a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,20 +48,22 @@ The streaming setup now lives in one shared place instead of being copied per tr so the NF4 pre-flight, the RAM/disk tier choice and the VRAM fit refusal cannot drift between SFT and the preference losses. -**Fixed — `trl` is now capped below 0.29, and that is a real bug fix, not a CI tweak.** -`trl` 0.29.0 removed `ORPOConfig` and `CPOConfig` outright and dropped -`max_prompt_length` from the remaining preference configs, so **six trainers** (`bco`, -`dpo`, `ipo`, `kto`, `orpo`, `simpo`) could not so much as build their configuration -against it: anyone who ran `pip install 'soup-cli[train]'` and resolved to 0.29+ had -`soup train --task orpo` fail on import. That was already true before this release and -nothing caught it — the `trl` imports live inside `setup()`, which no test had ever -called on those wrappers, so CI stayed green while the code only worked below 0.29. -This release's end-to-end preference tests are what surfaced it. +**Fixed — `trl` is now capped below 0.25, and that is a real bug fix, not a CI tweak.** +Six trainers (`bco`, `dpo`, `ipo`, `kto`, `orpo`, `simpo`) pass `max_prompt_length` to +their `trl` config, and `trl` removed it in stages: `bco` at 0.25, `kto`/`orpo`/`simpo` +at 0.26, `dpo`/`ipo` at 0.29 — which also deleted `ORPOConfig` and `CPOConfig` +outright. So anyone who ran `pip install 'soup-cli[train]'` and resolved to a recent +`trl` had `soup train --task orpo` fail on import. -The boundary was established by reading the published wheels rather than assumed: -0.28.0 still exports both configs and still accepts `max_prompt_length`; 0.29.0 has -none of the three. Supporting the newer API is its own piece of work; declaring a -dependency the code actually works with comes first. +That was already true before this release and nothing caught it: the `trl` imports +live inside `setup()`, which no test had ever called on those wrappers, so CI stayed +green while the code only worked on older `trl`. This release's end-to-end preference +tests are what surfaced it. + +The boundary was read off the published wheels per config rather than inferred from a +version number — the removal being staged is exactly why a single spot-check gives the +wrong answer. Supporting the newer API is its own piece of work; declaring a dependency +the code actually works with comes first. Honest costs: streaming makes the reference free in **memory**, not in **time** — DPO traverses the layer stack three times per step against SFT's two, measured at **1.52x** diff --git a/benchmarks/gate-v0.72.4-preference-losses.md b/benchmarks/gate-v0.72.4-preference-losses.md index 5ef4922..f265463 100644 --- a/benchmarks/gate-v0.72.4-preference-losses.md +++ b/benchmarks/gate-v0.72.4-preference-losses.md @@ -24,11 +24,29 @@ written before a gate passed. > therefore also records *which TRL internals the property depends on*, so the > shipped tests assert the property rather than the internals. > -> An earlier note in this repo claimed CI runs trl "1.9.2". That was wrong and -> repeating it cost a red CI cycle: the break is at a MINOR, 0.29.0, which -> removed `ORPOConfig`, `CPOConfig` and `max_prompt_length`. Established by -> reading the published wheels (0.28.0 has all three, 0.29.0 has none), not -> inferred from a major-version bump. The dependency is capped `<0.29`. +> **A dependency break found during this release, and two wrong diagnoses before +> the right one.** Six trainers (`bco`, `dpo`, `ipo`, `kto`, `orpo`, `simpo`) pass +> `max_prompt_length` to their trl config, and trl removed it in STAGES — which is +> why each spot-check gave a different answer. Read per config off the published +> wheels: +> +> | version | dpo | kto | orpo | cpo | bco | +> |---|---|---|---|---|---| +> | 0.24.0 | yes | yes | yes | yes | yes | +> | 0.25.1 | yes | yes | yes | yes | **NO** | +> | 0.26.0 | yes | **NO** | **NO** | **NO** | NO | +> | 0.29.0 | **NO** | NO | *gone* | *gone* | *gone* | +> +> First diagnosis: "trl 1.x removed them" — taken from an existing note in this +> repo claiming CI runs trl 1.9.2. Wrong; CI's install log shows **0.29.1**, so a +> `<1` cap excluded nothing. Second: "the break is 0.29.0" — right for `dpo`, but +> checked only `dpo_config.py` and extrapolated. The dependency is capped +> **`<0.25`**, the last release on which all six work. +> +> This is a pre-existing bug, not one this release introduced: the trl imports +> live inside `setup()`, which no test had ever called on those wrappers, so CI +> stayed green while `soup train --task orpo` was broken for anyone installing +> fresh. v0.72.4's end-to-end preference tests are what surfaced it. **The inherited standard** — a streamed run must be bit-exact against the resident run of the same numerics; what changes per slot is the *reference*, not diff --git a/pyproject.toml b/pyproject.toml index 4e37352..260d143 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,21 +41,26 @@ train = [ "torch>=2.0.0", "transformers>=4.36.0,<5.0.0", "peft>=0.7.0", - # v0.72.4: capped below 0.29. The break is at a MINOR, not at 1.0 — verified - # by reading the wheels rather than assuming: 0.28.0 still exports - # `ORPOConfig` and `CPOConfig` and still accepts `max_prompt_length`; 0.29.0 - # removes all three. Six trainers (bco, dpo, ipo, kto, orpo, simpo) pass - # `max_prompt_length`, and orpo/simpo import configs that no longer exist, so - # on >=0.29 `soup train --task orpo` fails at import. + # v0.72.4: capped below 0.25. Six trainers (bco, dpo, ipo, kto, orpo, simpo) + # pass `max_prompt_length` to their trl config, and trl removed it in STAGES + # rather than in one release — which is why a single spot-check gives the + # wrong answer. Read off the published wheels, per config: # - # That was latent, not new: the trl imports live inside `setup()`, which no - # test had ever called on those wrappers, so CI stayed green while the code - # only worked on <0.29 and anyone pip-installing today got a broken install. - # v0.72.4's end-to-end preference tests are what surfaced it. + # version dpo kto orpo cpo bco + # 0.24.0 yes yes yes yes yes <- last release all six work on + # 0.25.1 yes yes yes yes NO + # 0.26.0 yes NO NO NO NO + # 0.29.0 NO NO gone gone gone <- ORPOConfig/CPOConfig deleted # - # Migrating to the >=0.29 API is its own piece of work; shipping against a + # So `soup train --task bco` breaks at 0.25, `--task kto|orpo|simpo` at 0.26, + # and `--task dpo|ipo` at 0.29. That was latent, not new: the trl imports live + # inside `setup()`, which no test had ever called on those wrappers, so CI + # stayed green while anyone pip-installing got a broken install. v0.72.4's + # end-to-end preference tests are what surfaced it. + # + # Migrating to the newer API is its own piece of work; shipping against a # release nobody has validated is not a substitute for it. - "trl>=0.7.0,<0.29", + "trl>=0.7.0,<0.25", "datasets>=2.14.0", "bitsandbytes>=0.41.0", "accelerate>=0.25.0",