fix: set chat_template on inner tokenizer for multimodal models#1257
fix: set chat_template on inner tokenizer for multimodal models#1257hiworldwzj merged 2 commits intomainfrom
Conversation
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.
There was a problem hiding this comment.
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.
lightllm/server/build_prompt.py
Outdated
| # 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 |
There was a problem hiding this comment.
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_strFor 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.
Summary
--chat_templateis specified, the custom template was only set on the outerBaseMultiModalTokenizerwrapper but not on the inner HF tokenizerapply_chat_templatedelegates to the inner tokenizer via__getattr__, the custom template was silently ignored for multimodal models (Qwen-VL, LLaVA, InternVL, etc.)chat_templateon the inner tokenizer when it exists, consistent with the existing logic forchat_template.jsonloading (lines 37-41)Test plan
--chat_templateuse the custom template--chat_templatestill work as before