mirror of
https://github.com/unslothai/unsloth.git
synced 2026-07-10 00:08:58 +00:00
* Fix CPOTrainer crash with multimodal processors CPOTrainer shares build_tokenized_answer/tokenize_row and __init__ with ORPOTrainer, but the ORPO replacement functions that route tokenization through the underlying text tokenizer and resolve pad_token_id were only registered for orpo_trainer. With a multimodal processing class (e.g. Gemma4Processor) the positional self.processing_class(prompt, ...) call binds prompt to images=, leaving text=None and raising TypeError: 'NoneType' object is not subscriptable. Register the existing orpo_trainer_text_tokenizer and orpo_trainer_processor_pad_token under cpo_trainer as well so CPO/SimPO fine-tuning of multimodal models works. No change for plain tokenizers. * Add CPO processor tokenizer regression test Static, CPU-only checks that cpo_trainer registers the same orpo_trainer_text_tokenizer and orpo_trainer_processor_pad_token rewriters as orpo_trainer, and that the rewriter drops the broken positional self.processing_class(prompt, ...) call. Guards against issue #4952 regressing. * Format CPO test assert for ruff line length (pre-commit) * Bind CPO __init__ pad/eos token reads to underlying tokenizer TRL 0.28+ CPOTrainer.__init__ reads bare processing_class.pad_token and processing_class.eos_token before pad_token_id, which raises AttributeError for multimodal processors (e.g. Gemma) where those live on .tokenizer. Extend orpo_trainer_processor_pad_token to route that block through the underlying tokenizer, and add a regression test. * Tighten code comments (no logic change) * Make CPO/ORPO rewriters reach the trainer on TRL 1.x TRL 1.x moved CPOTrainer and ORPOTrainer out of trl.trainer into trl.experimental.<algo> and dropped the trl.trainer.<algo>_trainer shim that older TRL (0.26 - 0.28) kept. patch_trl_rl_trainers() discovers trainers via dir(trl.trainer), so on TRL 1.x cpo_trainer and orpo_trainer are never found and the multimodal-processor tokenization fix (#4952) silently stops applying, even though the rewriters themselves still match the source. Re-expose experimental-only trainers that Unsloth has rewriters for (RL_FUNCTIONS keys) under trl.trainer before discovery, so the existing patch machinery and its thin-wrapper resolution work unchanged. The alias is a no-op on older TRL where trl.trainer.<algo>_trainer already exists. Also rebind the patched Trainer/Config into every already-imported trl.* module that holds the original class so the fix is visible at the experimental import site (from trl.experimental.cpo import CPOTrainer), not only via trl.trainer. Verified on transformers 4.57.6 + trl 0.22.2, transformers 4.57.6 + trl 0.27.1, and transformers 5.12.1 + trl 1.6.0: CPOTrainer with a multimodal processor tokenizes through the underlying text tokenizer with no crash on all three, and the SFT/GRPO/DPO patch paths are unchanged. * Format for ruff (pre-commit) * Simplify CPO fix to mirror ORPO registrations (#4952) Register the existing ORPO row-tokenizer/pad-token rewriters for cpo_trainer. Under the trl<=0.24.0 pin CPOTrainer lives in trl.trainer.cpo_trainer (found by dir(trl.trainer)), shares ORPO's build_tokenized_answer and uses processing_class.pad_token_id, so the two registrations are sufficient. Drop the trl 1.x experimental aliasing/rebind machinery in rl.py and the bare-pad_token rewriter: trl 1.x (CPO in trl.experimental) and the bare pad_token pattern (trl>=0.28) are not installable under the pin. * CPO: route bare pad_token/eos_token default through inner tokenizer TRL 1.x CPO/ORPO __init__ (the trl.experimental source unsloth resolves on TRL 0.26+) defaults processing_class.pad_token from processing_class.eos_token before tokenizing. Multimodal processors (Gemma3/Gemma4 Processor) expose those attributes on .tokenizer, not on the processor, so that bare access raises AttributeError during __init__ even with the pad_token_id fallback registered. Extend orpo_trainer_processor_pad_token to rewrite that defaulting block to run on the inner tokenizer. The pinned TRL range (<=0.24.0) has no such block, so the regex is a no-op there and only the existing pad_token_id fallback applies. Verified the rewrite against the real trl 1.6.0 experimental CPOTrainer.__init__ (bare access removed, result compiles, a processor without pad_token no longer raises) and added offline regression tests for both the rewrite and its no-op. --------- Co-authored-by: Daniel Han <michaelhan2050@gmail.com>
99 lines
4 KiB
Python
99 lines
4 KiB
Python
"""CPO shares ORPO's row-tokenization replacements (issue #4952).
|
|
|
|
CPOTrainer reuses ORPO's tokenize/init code, so the ORPO rewriters must also be
|
|
registered for cpo_trainer. The rewriters themselves are covered by
|
|
test_orpo_processor_text_tokenizer.py; here we just check cpo mirrors orpo.
|
|
Static, CPU-only, no torch.
|
|
"""
|
|
|
|
import ast
|
|
import os
|
|
|
|
|
|
REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
|
|
RL_PATH = os.path.join(REPO_ROOT, "unsloth", "models", "rl_replacements.py")
|
|
|
|
|
|
def _registrations(source):
|
|
"""Map each RL_FUNCTIONS[key] target to the appended function names."""
|
|
out = {}
|
|
for node in ast.walk(ast.parse(source)):
|
|
if not isinstance(node, ast.Expr):
|
|
continue
|
|
call = node.value
|
|
if not (isinstance(call, ast.Call) and isinstance(call.func, ast.Attribute)):
|
|
continue
|
|
if call.func.attr != "append":
|
|
continue
|
|
sub = call.func.value
|
|
if not (isinstance(sub, ast.Subscript) and isinstance(sub.value, ast.Name)):
|
|
continue
|
|
if sub.value.id != "RL_FUNCTIONS":
|
|
continue
|
|
key = sub.slice
|
|
if not (isinstance(key, ast.Constant) and isinstance(key.value, str)):
|
|
continue
|
|
arg = call.args[0]
|
|
if isinstance(arg, ast.Name):
|
|
out.setdefault(key.value, []).append(arg.id)
|
|
return out
|
|
|
|
|
|
def test_cpo_registration_matches_orpo():
|
|
regs = _registrations(open(RL_PATH).read())
|
|
shared = {"orpo_trainer_text_tokenizer", "orpo_trainer_processor_pad_token"}
|
|
assert shared <= set(regs.get("orpo_trainer", []))
|
|
assert shared <= set(regs.get("cpo_trainer", []))
|
|
|
|
|
|
def _load_pad_rewriter():
|
|
"""Exec orpo_trainer_processor_pad_token (+ _PAD_FALLBACK) without importing unsloth."""
|
|
tree = ast.parse(open(RL_PATH).read())
|
|
nodes = []
|
|
for n in tree.body:
|
|
if isinstance(n, ast.Assign) and any(
|
|
getattr(t, "id", None) == "_PAD_FALLBACK" for t in n.targets
|
|
):
|
|
nodes.append(n)
|
|
elif isinstance(n, ast.FunctionDef) and n.name == "orpo_trainer_processor_pad_token":
|
|
nodes.append(n)
|
|
import re as _re
|
|
|
|
ns = {"re": _re}
|
|
exec(compile(ast.Module(body = nodes, type_ignores = []), RL_PATH, "exec"), ns)
|
|
return ns["orpo_trainer_processor_pad_token"]
|
|
|
|
|
|
def test_pad_token_default_routed_through_inner_tokenizer():
|
|
# TRL 1.x CPO/ORPO __init__ defaults pad_token from eos_token before
|
|
# tokenizing; on a multimodal processor those live on `.tokenizer`. The
|
|
# rewrite must route both the default and pad_token_id through the inner
|
|
# tokenizer so a processor without bare pad_token does not AttributeError.
|
|
rewrite = _load_pad_rewriter()
|
|
init_src = (
|
|
"def __init__(self, model, args, processing_class):\n"
|
|
" if processing_class.pad_token is None:\n"
|
|
" processing_class.pad_token = processing_class.eos_token\n"
|
|
" self.pad_token_id = processing_class.pad_token_id\n"
|
|
)
|
|
out = rewrite("__init__", init_src)
|
|
assert "if processing_class.pad_token is None:" not in out
|
|
assert "processing_class.pad_token = processing_class.eos_token" not in out
|
|
assert "_unsloth_proc_tok = getattr(processing_class, 'tokenizer', processing_class)" in out
|
|
# bare pad_token_id must be routed through the getattr fallback, not left raw
|
|
assert "= processing_class.pad_token_id\n" not in out
|
|
ast.parse(out) # rewritten source still compiles
|
|
|
|
|
|
def test_pad_rewrite_noop_without_bare_pad_block():
|
|
# Older TRL (the pinned <=0.24.0 range) has no bare pad_token block; the
|
|
# rewrite must only touch pad_token_id and leave everything else intact.
|
|
rewrite = _load_pad_rewriter()
|
|
init_src = (
|
|
"def __init__(self, model, args, processing_class):\n"
|
|
" self.pad_token_id = processing_class.pad_token_id\n"
|
|
)
|
|
out = rewrite("__init__", init_src)
|
|
assert "_unsloth_proc_tok" not in out
|
|
assert "= processing_class.pad_token_id\n" not in out # still routed via fallback
|
|
ast.parse(out)
|