allenai / allenai/open-instruct

`--chat_template_name` silently accepts unrecognised names and trains with a different template

Offen
#1,805 0 Kommentare 0 Reaktionen 1 zugewiesene Person Beansprucht von @farhatkevin Auf GitHub ansehen
Vorherrschende Sprache
Python
Sterne
3.9k
Forks
585
Ø Merge
5 T. 17 Std.
Gemergte PRs (30 T.)
16

Beschreibung

## Summary

An unrecognised `--chat_template_name` does not error. `get_tokenizer_tulu_v2_2` falls through to
the tokenizer's own `chat_template` (`dataset_transformation.py:694-700`):

```python
if tc.chat_template_name in CHAT_TEMPLATES:
tokenizer.chat_template = CHAT_TEMPLATES[tc.chat_template_name]
else:
try:
tokenizer.chat_template = AutoTokenizer.from_pretrained(
tc.tokenizer_name_or_path, revision=tc.tokenizer_revision
).chat_template
except Exception:
raise ValueError(f"Could not find chat template for {tc.tokenizer_name_or_path}.") from None
```

So a typo in the flag does not fail the run — it quietly changes which template you train with.
The error message on the failure path names the *tokenizer*, not the unrecognised template name,
which makes the situation harder to recognise even when it does raise.

## This is not hypothetical

Both shipped Olmo 3 SFT datasets took this path. Their `dataset_statistics.json` records:

| dataset | `chat_template` recorded | in `CHAT_TEMPLATES`? |
|---|---|---|
| `olmo3-32b-thinking-sft` | `olmo_thinker_no_think_final` | never — `git log -S` finds no commit introducing it |
| `olmo3-32b-instruct-sft-1114` | `olmo123` | never |

Both therefore trained with the chat template baked into their custom tokenizer
(`dolma2-tokenizer-lc-reasoner-FINAL`, `dolma2-tokenizer-no-system-prompt`) rather than anything
in this repo. `olmo123` is still referenced by ~10 current scripts under `scripts/train/olmo3/`
and `scripts/train/olmo-hybrid/`.

(The labels themselves were fine — those datasets were verified correctly masked, see #1800. The
problem is that you cannot tell what was applied from what was recorded.)

## Impact

- A typo silently changes training rather than failing fast.
- `dataset_statistics.json` records the *requested* name, not the *resolved* template, so a run
is not reproducible from its own statistics. Anyone reading those files reasonably concludes a
registry template was used.
- The fallback is genuinely useful — using a model's published template is a real workflow — but
it is currently indistinguishable from a mistake.

Caching is **not** affected: the dataset cache fingerprint already hashes the resolved template
content (`chat_template_hash`, `dataset_transformation.py:2024`), so a changed template does
correctly invalidate the cache.

## Suggested fix

1. Make the fallback explicit. Reserve a sentinel (e.g. `--chat_template_name tokenizer_default`)
for "use the tokenizer's own template", and raise on any other unrecognised name, listing the
available keys. Consider a fuzzy-match hint, since `olmo123` and
`olmo_thinker_no_think_final` both look like intended registry names.
2. Record the resolved template in `dataset_statistics.json` — its hash at minimum, ideally the
source (`registry:` vs `tokenizer:`) — so a dataset documents what it was actually
built with.
3. Audit the ~10 scripts still passing `olmo123` and either point them at a real registry entry
or at the explicit sentinel.

Step 1 is breaking for those scripts, which is arguably the point — they are currently not
training with the template their command line claims.

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.