Skip to content

Fix dataset fingerprinting in experimental KTO and TPO trainers#6233

Closed
DaoyuanLi2816 wants to merge 1 commit into
huggingface:mainfrom
DaoyuanLi2816:fix/kto-tpo-dataset-fingerprinting
Closed

Fix dataset fingerprinting in experimental KTO and TPO trainers#6233
DaoyuanLi2816 wants to merge 1 commit into
huggingface:mainfrom
DaoyuanLi2816:fix/kto-tpo-dataset-fingerprinting

Conversation

@DaoyuanLi2816

@DaoyuanLi2816 DaoyuanLi2816 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #6206, which fixed dataset fingerprinting in the core DPOTrainer / SFTTrainer / RewardTrainer. The two experimental trainers KTOTrainer and TPOTrainer still have the pattern #6206 removed: their tokenization map is a closure that calls self._tokenize(...), so it captures self. A map function that closes over self (which holds the model and other unpicklable state) can't be hashed stably, so datasets falls back to a random fingerprint and silently disables caching of the tokenized dataset — re-tokenizing on every run.

This applies the same fix as #6206: make _tokenize a @staticmethod and bind it to a local (tokenize = self._tokenize) before defining the map function, so the closure no longer captures self.

No behavior change: the tokenized output is identical; only the map function's hashability (and therefore cache reuse across runs) changes.

CPOTrainer and ORPOTrainer share the same pattern (a self-bound tokenize_row) but are intentionally left out here to avoid conflicts: CPO's data pipeline is being refactored in #5837, and ORPO's _prepare_dataset is touched by #6230. They can be aligned once those land.

Verification

  • tests/experimental/test_kto_trainer.py and tests/experimental/test_tpo_trainer.py pass in full (tokenized output unchanged); ruff check / ruff format --check clean. Validated on a single GPU (RTX 4080).
  • Like Fix dataset fingerprinting in DPO/SFT tokenization #6206, this doesn't add a fingerprint-specific test: the fallback to a random fingerprint only triggers for processing classes / models that datasets can't hash, which the tiny CI fixtures don't exercise. The existing suites cover tokenization correctness.

Note

Low Risk
Refactor limited to dataset preprocessing closures with no training-loss or tokenization logic changes; behavior is intended to be identical aside from caching.

Overview
Aligns experimental KTOTrainer and TPOTrainer with the dataset-fingerprint fix from #6206 so dataset.map during preprocessing can be cached across runs instead of re-tokenizing every time.

In both trainers, _tokenize is now a @staticmethod, and tokenize_fn calls a local tokenize = self._tokenize so the map callback no longer closes over self (which held the model and broke stable hashing). KTO also threads is_vlm as an explicit argument via fn_kwargs, matching DPO’s VLM handling.

Tokenized outputs are unchanged; only cache fingerprint stability improves.

Reviewed by Cursor Bugbot for commit bf7b8e4. Bugbot is set up for automated code reviews on this repo. Configure here.

Follow-up to huggingface#6206, which fixed the same issue in DPO/SFT/Reward. `KTOTrainer`
and `TPOTrainer` tokenize their datasets with a `map` function that closes over
`self` (it calls `self._tokenize(...)`). A closure over `self` — which holds the
model and other unpicklable state — can't be hashed stably, so `datasets` falls
back to a random fingerprint and silently disables caching of the tokenized
dataset.

Mirror huggingface#6206: make `_tokenize` a `@staticmethod` and bind it to a local before
defining the map function, so the closure no longer captures `self`. KTO passes
the VLM flag through as an explicit `is_vlm` parameter; TPO's `_tokenize` does
not use `self`, so it simply becomes a staticmethod. The tokenized output is
unchanged.
@albertvillanova

Copy link
Copy Markdown
Member

Duplicate of:

@DaoyuanLi2816
DaoyuanLi2816 deleted the fix/kto-tpo-dataset-fingerprinting branch July 2, 2026 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants