mirror of https://github.com/razor-ai/soup.git
fix(sft): mirror vision-processor pad_token from nested tokenizer (#302)
SmolVLM/Idefics3 vision SFT crashed with 'Idefics3Processor object has no attribute pad_token': HF vision processors keep the text tokenizer nested at processor.tokenizer and don't forward token-level attributes (ProcessorMixin has no __getattr__), but TRL's SFTTrainer reads processing_class.pad_token / .eos_token / .convert_tokens_to_ids directly. Add _ensure_vision_processor_pad_token: set pad_token = eos_token on the inner tokenizer when unset, then mirror the token surface + convert_tokens_to_ids onto the processor — only for attributes it doesn't already expose, so a LLaVA-style or tokenizer-like processing_class is untouched (no regression). Verified live on SmolVLM-256M (RTX 3050): setup, tokenization, and PAD/BOS/EOS alignment now succeed. A full training STEP still needs Idefics3-aware vision collation (pixel_values + image-token expansion) — the LLaVA-era path pre-renders text and never builds pixel_values, so Idefics3.forward gets 3D input_ids. That collation rework is left open under #302; the recipe stays parse-only with an updated note.
This commit is contained in:
Alpamys
parent
e9266825e3
commit
481dc388d3
|
|
@ -2284,10 +2284,12 @@ output: ./output
|
|||
size="256M",
|
||||
tags=("smolvlm", "vision", "multimodal", "vlm", "sft", "tiny", "edge"),
|
||||
description="SmolVLM 256M vision SFT (llava format) — a tiny VLM. NOTE: "
|
||||
"SmolVLM uses an Idefics3 processor; live vision SFT needs Idefics3 "
|
||||
"vision-path support (the LLaVA path assumes a different processor "
|
||||
"API) — tracked as a follow-up. target_modules pinned to q_proj/v_proj "
|
||||
"(auto cannot infer them for Idefics3).",
|
||||
"SmolVLM uses an Idefics3 processor. The processor pad_token blocker is "
|
||||
"fixed (#302 — the nested tokenizer's token surface is mirrored onto the "
|
||||
"processor), so setup + tokenization now run; a full training STEP still "
|
||||
"needs Idefics3-aware vision collation (pixel_values + image-token "
|
||||
"expansion) — parse-tested for now, tracked in #302. target_modules "
|
||||
"pinned to q_proj/v_proj (auto cannot infer them for Idefics3).",
|
||||
yaml_str="""\
|
||||
base: HuggingFaceTB/SmolVLM-256M-Instruct
|
||||
task: sft
|
||||
|
|
|
|||
|
|
@ -16,6 +16,64 @@ logger = logging.getLogger(__name__)
|
|||
|
||||
console = Console()
|
||||
|
||||
# Text-token surface TRL's SFTTrainer reads directly off ``processing_class``
|
||||
# (trl/trainer/sft_trainer.py: ``pad_token`` / ``eos_token`` / ``eos_token_id``
|
||||
# resolution) — mirrored from a vision processor's nested tokenizer in #302.
|
||||
_PROCESSOR_TOKEN_ATTRS = (
|
||||
"pad_token",
|
||||
"eos_token",
|
||||
"pad_token_id",
|
||||
"eos_token_id",
|
||||
"bos_token",
|
||||
"bos_token_id",
|
||||
)
|
||||
|
||||
|
||||
def _ensure_vision_processor_pad_token(processor: object) -> None:
|
||||
"""Mirror a vision processor's nested-tokenizer token surface onto itself.
|
||||
|
||||
HF vision processors (Idefics3/SmolVLM, LLaVA, Qwen2-VL, ...) keep the text
|
||||
tokenizer nested at ``processor.tokenizer`` and do NOT forward token-level
|
||||
attributes — ``ProcessorMixin`` has no ``__getattr__``. TRL's ``SFTTrainer``
|
||||
reads ``processing_class.pad_token`` / ``.eos_token`` /
|
||||
``.convert_tokens_to_ids`` directly, so passing such a processor as
|
||||
``processing_class`` crashes with e.g. ``'Idefics3Processor' object has no
|
||||
attribute 'pad_token'`` (#302).
|
||||
|
||||
Fix: when the processor exposes a nested ``.tokenizer``, set
|
||||
``pad_token = eos_token`` on that tokenizer if unset, then copy the token
|
||||
surface + ``convert_tokens_to_ids`` onto the processor — but only for
|
||||
attributes it does not already expose, so a processor that already behaves
|
||||
like a tokenizer (or a plain tokenizer) is left untouched (no LLaVA-path
|
||||
regression). Best-effort per attribute: a read-only property on either side
|
||||
is skipped rather than fatal.
|
||||
"""
|
||||
tok = getattr(processor, "tokenizer", None)
|
||||
if tok is None:
|
||||
# Already tokenizer-like, or an unknown shape — nothing to mirror.
|
||||
return
|
||||
# A padless tokenizer trains fine once pad == eos (the standard causal-LM
|
||||
# convention already used by the text path, sft.py:_setup_transformers).
|
||||
if getattr(tok, "pad_token", None) is None and getattr(tok, "eos_token", None) is not None:
|
||||
try:
|
||||
tok.pad_token = tok.eos_token
|
||||
except (AttributeError, TypeError):
|
||||
pass
|
||||
for attr in _PROCESSOR_TOKEN_ATTRS:
|
||||
if hasattr(processor, attr):
|
||||
continue # processor already exposes it — don't clobber
|
||||
try:
|
||||
setattr(processor, attr, getattr(tok, attr, None))
|
||||
except (AttributeError, TypeError):
|
||||
pass
|
||||
if not hasattr(processor, "convert_tokens_to_ids"):
|
||||
inner = getattr(tok, "convert_tokens_to_ids", None)
|
||||
if callable(inner):
|
||||
try:
|
||||
processor.convert_tokens_to_ids = inner
|
||||
except (AttributeError, TypeError):
|
||||
pass
|
||||
|
||||
|
||||
def _maybe_load_pretokenized(
|
||||
dcfg, base: str, console_obj: Console,
|
||||
|
|
@ -929,6 +987,10 @@ class SFTTrainerWrapper:
|
|||
self.processor = AutoProcessor.from_pretrained(
|
||||
cfg.base, trust_remote_code=self._trust_remote_code
|
||||
)
|
||||
# Idefics3/SmolVLM (and other) processors keep the text tokenizer nested
|
||||
# and don't forward pad_token/eos_token — mirror them onto the processor
|
||||
# so TRL's SFTTrainer processing_class access doesn't crash (#302).
|
||||
_ensure_vision_processor_pad_token(self.processor)
|
||||
self.tokenizer = self.processor # SFTTrainer uses processing_class
|
||||
|
||||
# Quantization (v0.71.19 #81) — unified Quant Menu loader. Replaces the
|
||||
|
|
|
|||
|
|
@ -0,0 +1,214 @@
|
|||
"""Issue #302 — Idefics3 / SmolVLM vision-SFT pad_token routing.
|
||||
|
||||
SmolVLM uses an ``Idefics3Processor``. The shared LLaVA vision path sets
|
||||
``self.tokenizer = <processor>`` and hands it to TRL's ``SFTTrainer`` as
|
||||
``processing_class``. TRL reads ``processing_class.pad_token`` /
|
||||
``.eos_token`` / ``.convert_tokens_to_ids`` directly, but HF vision processors
|
||||
keep the text tokenizer nested at ``processor.tokenizer`` and do NOT forward
|
||||
token-level attributes (``ProcessorMixin`` has no ``__getattr__``) — so training
|
||||
crashes with ``AttributeError: 'Idefics3Processor' object has no attribute
|
||||
'pad_token'``.
|
||||
|
||||
This suite pins ``_ensure_vision_processor_pad_token`` — it mirrors the inner
|
||||
tokenizer's text-token surface onto the processor (setting pad_token = eos_token
|
||||
when unset), reproducing TRL's exact ``args.pad_token or processing_class.pad_token
|
||||
or processing_class.eos_token`` access. Both Idefics3 and LLaVA processors share
|
||||
identical structure (``attributes = ['image_processor', 'tokenizer']``), so the
|
||||
fix repairs both without regressing a processor that already exposes pad_token.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
class _FakeTokenizer:
|
||||
"""Text tokenizer with the token surface TRL reads off processing_class."""
|
||||
|
||||
def __init__(self, pad_token=None, eos_token="</s>"):
|
||||
self.pad_token = pad_token
|
||||
self.eos_token = eos_token
|
||||
self.eos_token_id = 2
|
||||
self.bos_token = "<s>"
|
||||
self.bos_token_id = 1
|
||||
|
||||
@property
|
||||
def pad_token_id(self):
|
||||
# Mirrors a real tokenizer: None until pad_token is set.
|
||||
return 0 if self.pad_token is not None else None
|
||||
|
||||
def convert_tokens_to_ids(self, token):
|
||||
return {"</s>": 2, "<s>": 1, "<pad>": 0}.get(token, 2)
|
||||
|
||||
|
||||
class _FakeIdefics3Processor:
|
||||
"""Mimics Idefics3Processor: nested .tokenizer, NO pad_token forwarding."""
|
||||
|
||||
attributes = ["image_processor", "tokenizer"]
|
||||
|
||||
def __init__(self, tokenizer):
|
||||
self.tokenizer = tokenizer
|
||||
self.image_processor = object()
|
||||
|
||||
# No __getattr__ — accessing .pad_token raises AttributeError, exactly like
|
||||
# the real ProcessorMixin subclass.
|
||||
|
||||
|
||||
class _TokenizerLikeProcessor:
|
||||
"""A processing_class that already exposes the token surface (regression guard)."""
|
||||
|
||||
def __init__(self):
|
||||
self.pad_token = "<pad>"
|
||||
self.eos_token = "</s>"
|
||||
self.tokenizer = None
|
||||
|
||||
def convert_tokens_to_ids(self, token):
|
||||
return 0
|
||||
|
||||
|
||||
def _trl_pad_token(processing_class, args_pad_token=None):
|
||||
"""Reproduce TRL SFTTrainer's pad-token resolution (sft_trainer.py:436)."""
|
||||
return args_pad_token or processing_class.pad_token or processing_class.eos_token
|
||||
|
||||
|
||||
class TestEnsureVisionProcessorPadToken:
|
||||
def test_bare_processor_raises_before_fix(self):
|
||||
# Sanity: the un-fixed processor reproduces the reported AttributeError.
|
||||
proc = _FakeIdefics3Processor(_FakeTokenizer(pad_token=None))
|
||||
with pytest.raises(AttributeError):
|
||||
_ = proc.pad_token
|
||||
|
||||
def test_sets_pad_token_from_eos(self):
|
||||
from soup_cli.trainer.sft import _ensure_vision_processor_pad_token
|
||||
|
||||
proc = _FakeIdefics3Processor(_FakeTokenizer(pad_token=None, eos_token="</s>"))
|
||||
_ensure_vision_processor_pad_token(proc)
|
||||
# Inner tokenizer got pad_token = eos_token
|
||||
assert proc.tokenizer.pad_token == "</s>"
|
||||
# Processor now exposes the surface TRL reads
|
||||
assert proc.pad_token == "</s>"
|
||||
assert proc.eos_token == "</s>"
|
||||
|
||||
def test_trl_resolution_no_longer_crashes(self):
|
||||
from soup_cli.trainer.sft import _ensure_vision_processor_pad_token
|
||||
|
||||
proc = _FakeIdefics3Processor(_FakeTokenizer(pad_token=None))
|
||||
_ensure_vision_processor_pad_token(proc)
|
||||
pad = _trl_pad_token(proc)
|
||||
assert pad == "</s>"
|
||||
# convert_tokens_to_ids delegates to the inner tokenizer
|
||||
assert proc.convert_tokens_to_ids(pad) == 2
|
||||
|
||||
def test_preserves_existing_pad_token(self):
|
||||
from soup_cli.trainer.sft import _ensure_vision_processor_pad_token
|
||||
|
||||
tok = _FakeTokenizer(pad_token="<pad>", eos_token="</s>")
|
||||
proc = _FakeIdefics3Processor(tok)
|
||||
_ensure_vision_processor_pad_token(proc)
|
||||
assert proc.tokenizer.pad_token == "<pad>" # untouched
|
||||
assert proc.pad_token == "<pad>"
|
||||
|
||||
def test_tokenizer_like_processor_unchanged(self):
|
||||
# A processing_class that already exposes pad_token must not be clobbered.
|
||||
from soup_cli.trainer.sft import _ensure_vision_processor_pad_token
|
||||
|
||||
proc = _TokenizerLikeProcessor()
|
||||
_ensure_vision_processor_pad_token(proc)
|
||||
assert proc.pad_token == "<pad>"
|
||||
|
||||
def test_no_nested_tokenizer_is_noop(self):
|
||||
from soup_cli.trainer.sft import _ensure_vision_processor_pad_token
|
||||
|
||||
class _NoTok:
|
||||
pad_token = "<pad>"
|
||||
eos_token = "</s>"
|
||||
|
||||
proc = _NoTok()
|
||||
_ensure_vision_processor_pad_token(proc) # must not raise
|
||||
assert proc.pad_token == "<pad>"
|
||||
|
||||
def test_convert_tokens_to_ids_mirrored(self):
|
||||
from soup_cli.trainer.sft import _ensure_vision_processor_pad_token
|
||||
|
||||
proc = _FakeIdefics3Processor(_FakeTokenizer(pad_token=None))
|
||||
_ensure_vision_processor_pad_token(proc)
|
||||
assert callable(proc.convert_tokens_to_ids)
|
||||
assert proc.convert_tokens_to_ids("</s>") == 2
|
||||
|
||||
def test_eos_token_id_mirrored(self):
|
||||
from soup_cli.trainer.sft import _ensure_vision_processor_pad_token
|
||||
|
||||
proc = _FakeIdefics3Processor(_FakeTokenizer(pad_token=None))
|
||||
_ensure_vision_processor_pad_token(proc)
|
||||
assert proc.eos_token_id == 2
|
||||
|
||||
def test_pad_token_id_mirrored(self):
|
||||
from soup_cli.trainer.sft import _ensure_vision_processor_pad_token
|
||||
|
||||
proc = _FakeIdefics3Processor(_FakeTokenizer(pad_token=None))
|
||||
_ensure_vision_processor_pad_token(proc)
|
||||
# inner tokenizer's pad_token_id property becomes 0 once pad is set
|
||||
assert proc.pad_token_id == 0
|
||||
|
||||
def test_readonly_attr_degrades_gracefully(self):
|
||||
# A processor whose attributes can't be set (e.g. __slots__) must not
|
||||
# make the helper raise — the try/except degrades gracefully.
|
||||
from soup_cli.trainer.sft import _ensure_vision_processor_pad_token
|
||||
|
||||
class _SlotsProcessor:
|
||||
__slots__ = ("tokenizer",)
|
||||
|
||||
def __init__(self, tok):
|
||||
self.tokenizer = tok
|
||||
|
||||
proc = _SlotsProcessor(_FakeTokenizer(pad_token=None))
|
||||
# Must not raise even though setattr(proc, "pad_token", ...) fails.
|
||||
_ensure_vision_processor_pad_token(proc)
|
||||
# Inner tokenizer was still repaired (pad = eos).
|
||||
assert proc.tokenizer.pad_token == "</s>"
|
||||
|
||||
|
||||
class TestVisionSetupWiring:
|
||||
def test_setup_vision_transformers_invokes_pad_token_mirror(self, monkeypatch):
|
||||
# The fix is only useful if _setup_vision_transformers actually calls it.
|
||||
# Mock the heavy loads; assert the processor gets pad_token mirrored.
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import transformers
|
||||
|
||||
from soup_cli.config.loader import load_config_from_string
|
||||
from soup_cli.trainer.sft import SFTTrainerWrapper
|
||||
|
||||
fake_proc = _FakeIdefics3Processor(_FakeTokenizer(pad_token=None))
|
||||
monkeypatch.setattr(
|
||||
transformers.AutoProcessor, "from_pretrained",
|
||||
lambda *a, **k: fake_proc,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
transformers.AutoModelForVision2Seq, "from_pretrained",
|
||||
lambda *a, **k: MagicMock(),
|
||||
)
|
||||
import peft
|
||||
|
||||
monkeypatch.setattr(peft, "get_peft_model", lambda model, cfg: model)
|
||||
monkeypatch.setattr(
|
||||
"soup_cli.utils.quant_menu.build_quantization_config_for_loader",
|
||||
lambda **k: None,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"soup_cli.utils.data_pipeline.apply_vocab_expansion",
|
||||
lambda *a, **k: None,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
SFTTrainerWrapper, "_apply_quantization_aware", lambda self, tcfg: None
|
||||
)
|
||||
|
||||
cfg = load_config_from_string(
|
||||
"base: fake/vlm\ntask: sft\nmodality: vision\n"
|
||||
"data:\n train: x.jsonl\n format: llava\n max_length: 64\n"
|
||||
"training:\n quantization: none\n lora:\n target_modules: [q_proj, v_proj]\n"
|
||||
)
|
||||
wrapper = SFTTrainerWrapper(cfg, device="cpu")
|
||||
wrapper._setup_vision_transformers(cfg, cfg.training)
|
||||
# The helper ran: the Idefics3-style processor now exposes pad_token.
|
||||
assert wrapper.processor.pad_token == "</s>"
|
||||
Loading…
Reference in New Issue