Skip to content

fix: set chat_template on inner tokenizer for multimodal models#1257

Merged
hiworldwzj merged 2 commits intomainfrom
fix/multimodal-chat-template
Apr 7, 2026
Merged

fix: set chat_template on inner tokenizer for multimodal models#1257
hiworldwzj merged 2 commits intomainfrom
fix/multimodal-chat-template

Conversation

@sufubao
Copy link
Copy Markdown
Collaborator

@sufubao sufubao commented Apr 6, 2026

Summary

  • When --chat_template is specified, the custom template was only set on the outer BaseMultiModalTokenizer wrapper but not on the inner HF tokenizer
  • Since apply_chat_template delegates to the inner tokenizer via __getattr__, the custom template was silently ignored for multimodal models (Qwen-VL, LLaVA, InternVL, etc.)
  • This fix also sets chat_template on the inner tokenizer when it exists, consistent with the existing logic for chat_template.json loading (lines 37-41)

Test plan

  • Verify multimodal models (e.g. Qwen2-VL) with --chat_template use the custom template
  • Verify non-multimodal models with --chat_template still work as before

When --chat_template is specified, the template was only set on the
outer wrapper (BaseMultiModalTokenizer) but not on the inner HF
tokenizer. Since apply_chat_template delegates to the inner tokenizer
via __getattr__, the custom template was silently ignored for
multimodal models.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the init_tokenizer function to ensure chat templates are correctly applied to multimodal tokenizers by setting the template on the inner tokenizer object if it exists. A review comment suggests refactoring the implementation to use an if/else structure to maintain consistency with existing logic in the same file and avoid redundant attribute assignments.

Comment on lines +20 to +23
# Multimodal tokenizers wrap an inner HF tokenizer; apply_chat_template
# reads the template from the inner tokenizer, so set it there too.
if hasattr(tokenizer, "tokenizer"):
tokenizer.tokenizer.chat_template = chat_template_str
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.

medium

While this change correctly addresses the issue where the chat template was ignored for multimodal models, the implementation is slightly inconsistent with the logic used for chat_template.json later in the same function (lines 37-41).

Currently, line 19 sets chat_template on the wrapper object regardless of whether it is a multimodal tokenizer. Then, lines 22-23 set it again on the inner tokenizer. In contrast, the logic at lines 37-41 uses an if/else structure to set it either on the inner tokenizer or the wrapper.

To maintain consistency and avoid redundant attribute assignment (and potential shadowing of the inner tokenizer's attribute), consider refactoring this section to match the established pattern:

if hasattr(tokenizer, "tokenizer"):
    tokenizer.tokenizer.chat_template = chat_template_str
else:
    tokenizer.chat_template = chat_template_str

For multimodal models, set chat_template on the inner HF tokenizer.
For standard models, set it directly. Use if/else instead of always
setting on both levels.
@hiworldwzj hiworldwzj merged commit 40d8fdc into main Apr 7, 2026
1 check passed
@hiworldwzj hiworldwzj deleted the fix/multimodal-chat-template branch April 7, 2026 13:47
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.

2 participants