Skip to content

Fix IOIDataset BOS handling - #1773

Merged
jlarson4 merged 2 commits into
TransformerLensOrg:devfrom
emerardd:fix/ioi-dataset-double-bos
Sep 12, 2026
Merged

Fix IOIDataset BOS handling#1773
jlarson4 merged 2 commits into
TransformerLensOrg:devfrom
emerardd:fix/ioi-dataset-double-bos

Conversation

@emerardd

Copy link
Copy Markdown
Contributor

Description

IOIDataset previously called tokenizer.encode() with automatic special-token insertion enabled and then manually prepended a BOS. TransformerLens tokenizers configured to add BOS therefore produced two leading BOS tokens by default, while prepend_bos=False still left one BOS in place. Automatic BOS tokens in the IO/S label encodings also shifted the first differing-token index used by ioi_eval(), causing the default evaluation path to read logits one position too late.

This change encodes prompts and labels with add_special_tokens=False, leaving the existing prepend_bos branch as the sole source of a prompt BOS. The parameter documentation now states that contract.

Regression tests model a tokenizer that automatically inserts BOS, cover both values of prepend_bos, verify that IO/S contain only content tokens, and use position-sensitive logits to confirm that ioi_eval() reads the prediction immediately before the first answer token.

No new dependencies are required.

Fixes #1772

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Screenshots

Not applicable.

Validation

  • uv run --no-cache --no-sync pytest tests/unit/test_evals_ioi.py -q — 7 passed
  • uv run --no-cache --no-sync mypy . — success across 398 source files
  • pycln, isort, Black, and git diff --check — passed for the affected surface

The complete unit-test suite was not run locally.

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes — only the affected unit-test file was run
  • I have not rewritten tests relating to key interfaces which would affect backward compatibility

@koriyoshi2041 koriyoshi2041 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix matches the tokenizer contract: prompt and label encodings now exclude automatic special tokens, so is the only prompt BOS path and compares the first answer token. I reran the focused unit file at this exact head (7 passed) and ; both passed.

@koriyoshi2041

Copy link
Copy Markdown
Contributor

Correction to my approval wording: prepend_bos is the only prompt BOS path, and ioi_eval compares the first answer token. The focused unit file passed 7/7 at 54a3727d; git diff --check also passed.

@jlarson4

Copy link
Copy Markdown
Collaborator

Looks good and covers the reported issues, thanks @emerardd

@jlarson4
jlarson4 merged commit be40a9b into TransformerLensOrg:dev Sep 12, 2026
26 checks passed
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.

3 participants