allenai / allenai/open-instruct
Numpy SFT cache is not invalidated by changes to label derivation
- Lingua principale
- Python
- Stelle
- 3.9k
- Fork
- 585
- Merge medio
- 5g 17h
- PR unite (30g)
- 16
Descrizione
The numpy SFT cache key describes the *configuration* that produced the labels, but not the *code*:
```python
cache_hash = compute_config_hash(dcs, tc) # chat template, transform fns, tokenizer
seed_suffix = _seed_cache_suffix(seed, max_seq_length)
numpy_dir = f"{local_cache_dir}/{_NUMPY_SFT_SUBDIR}/{cache_hash}-{seed_suffix}"
```
(`olmo_core_finetune.py:135-137`, plus `DATASET_CACHE_VERSION` inside `compute_config_hash`.)
So a change to assistant-label derivation does not invalidate anything. `_numpy_dir_is_populated(numpy_dir)` returns True and the run reuses `labels_mask_part_*.npy` produced by the older code, with no warning.
## Why this matters now
#1806 changes which tokens are trainable. Anyone who lands it and re-runs against an existing cache trains on the **old** labels and sees no indication. The failure is silent and the artifacts look normal — the numpy files are present and correctly shaped, they just encode the previous masking.
Concretely: a validation run for #1806 launched against the full `tulu-3-sft-olmo-2-mixture` completed in ~40 s with 0 rows processed and no dataset statistics, because the key matched a cache built before the fix. Reported as "0 dropped rows" it looks like an improvement over the previous 15. It was zero work. Forcing a different `--seed` was the only way to get a real measurement.
## Relation to #1801
#1801 fixes the mirror case: on a cache **miss**, the printed remediation command omitted cache-key args, so following it re-tokenized under a different key and failed identically. Same root cause — the key is subtle and under-documented — but the failure modes differ in severity. A false miss fails loudly. A false hit silently serves stale labels.
## Options
1. **Mix the code version into the key.** Hash the source of the transform functions, or bump `DATASET_CACHE_VERSION` whenever label derivation changes. The latter is already the convention, just easy to forget — nothing enforces it.
2. **Record provenance and warn.** Write the git commit into the cache directory alongside `dataset_statistics.json`, and log loudly when reusing a cache built by a different commit. Cheaper, does not force re-tokenization, and would have made the situation above obvious.
3. **Document it.** At minimum note in `AGENTS.md` that changing tokenization requires bumping `DATASET_CACHE_VERSION` or using a fresh `--seed`.
(2) plus (3) seems like the best return: no forced re-tokenization on unrelated changes, but a stale cache stops being invisible.
Worth noting `DATASET_CACHE_VERSION` was not bumped in #1806 — deliberately, since the mixture-level output is unchanged for the overwhelming majority of rows, but that is exactly the judgement call this issue is about.
Guida per i contributori
Apri la guida per i contributori
Valutazione
Questa issue non è ancora stata valutata.