Skip token type ids in Hugging Face backend - #2619
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The change only covers one generation path, but other generation/tokenization paths in the same module still forward all tokenizer outputs and may continue to pass token_type_ids into model calls.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the HuggingFace v4.33+ chat-template generation path to explicitly skip token_type_ids during tokenization, and adds a unit test assertion to enforce that behavior.
Changes:
- Pass
return_token_type_ids=Falsetotokenizer.batch_encode_plus(...)inHuggingFacewithChatTemplate.generate. - Extend
test_generate_basicto assertreturn_token_type_idsis disabled in the tokenizer call.
File summaries
| File | Description |
|---|---|
| tests/models/test_huggingface_above_v4_33.py | Adds an assertion that batch_encode_plus is called with return_token_type_ids=False. |
| opencompass/models/huggingface_above_v4_33.py | Disables token_type_ids in the chat-template generate tokenization kwargs. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return_tensors='pt', | ||
| padding=True, | ||
| truncation=True, | ||
| add_special_tokens=True, | ||
| return_token_type_ids=False, |
There was a problem hiding this comment.
Updated the remaining generation and PPL paths with tests.
01cb666 to
02a48c3
Compare
|
Updated the base generation and PPL tokenization paths too. |
Motivation
Some
PreTrainedTokenizerFasttokenizers returntoken_type_ids. Hugging Face generation rejects this key for models that do not accept it, causing evaluation to fail.Modification
Disable
token_type_idsin Hugging Face generation and perplexity tokenization paths, with regression tests for both paths.Fixes #1533
Checklist