Fix: respect tokenizer_id over sibling tokenizer/ directory#787
Fix: respect tokenizer_id over sibling tokenizer/ directory#787rcvtkemgtpguv wants to merge 2 commits into
Conversation
Explicit -t/--tokenizer was ignored when a sibling tokenizer/ folder existed next to the checkpoint (common for SFT runs). Prefer the explicit tokenizer_id first, matching expected CLI behavior. Fixes allenai#609. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ec54dcb69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "olmo_core.nn.hf.convert_checkpoint.AutoTokenizer.from_pretrained", | ||
| return_value=fake_tok, | ||
| ) as from_pretrained, | ||
| patch("olmo_core.nn.hf.convert_checkpoint.save_hf_model"), |
There was a problem hiding this comment.
When either new test runs, this mock prevents save_hf_model from writing config.json, and the mocked tokenizer's save_pretrained also writes nothing. convert_checkpoint_to_hf unconditionally calls AutoConfig.from_pretrained(output_path) after saving (even with validate=False), so it raises before reaching the assertions; this makes both the regular Test job and Test examples job fail. Patch AutoConfig too or have the mocked save create a minimal config.
Useful? React with 👍 / 👎.
|
Closing for now — may reopen later. |
1 participant
Summary
convert_checkpoint_to_hfhonor an explicittokenizer_id(-t/--tokenizer) even when a siblingtokenizer/directory exists.Fixes #609.
Supersedes conflicting #629 (same intent; rebased onto current
convert_checkpoint.pylocation).Test plan
pytest src/test/examples/huggingface/test_convert_checkpoint_tokenizer.py../tokenizer/while passing-t <other-id>and confirm HF tokenizer files come from-t