Skip to content

fix wrong template in GLM4-0414 #13140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 27, 2025
Merged

fix wrong template in GLM4-0414 #13140

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4e0c059
fix wrong template in GLM4-0414
matteoserva Apr 24, 2025
0cce580
fix spaces
matteoserva Apr 24, 2025
cc93292
no bos token since it is already in the template
matteoserva Apr 24, 2025
36f927f
moved the chatgml4 check to higher priority
matteoserva Apr 27, 2025
f44e24b
restored template for old GLM models
matteoserva Apr 27, 2025
0128f14
moved the GLM4 template check in the correct place with correct check
matteoserva Apr 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion convert_hf_to_gguf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5154,7 +5154,7 @@ def set_vocab(self):
special_vocab._set_special_token("eos", tokenizer.get_added_vocab()["<|endoftext|>"])
special_vocab._set_special_token("eot", tokenizer.get_added_vocab()["<|user|>"])
special_vocab._set_special_token("unk", tokenizer.get_added_vocab()["<|endoftext|>"])
special_vocab._set_special_token("bos", tokenizer.get_added_vocab()["[gMASK]"])
special_vocab._set_special_token("bos", tokenizer.get_added_vocab()["<|endoftext|>"])
Copy link
Collaborator

Choose a reason for hiding this comment

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

#13099 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I can do that.

  • add_bos_token is already set to False by the HF model and conversion script.
  • The jinja templating system always removes the bos token, ignoring add_bos_token. So the prefix is removed and never added again.
  • If I remove the bos token altogether, the /props endpoint crashes because it expects it

This is the relevant jinja template code. reached when ./llama-server --jinja

llama.cpp/common/chat.cpp

Line 651 in e291450

if (string_starts_with(result, tmpl.bos_token())) {

Copy link
Collaborator

@ngxson ngxson Apr 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

if the BOS is not added, then what is the reason behind changing it from [gMASK] to <|endoftext|>? it shouldn't crash /props, right?

Nvm I see what you mean

special_vocab.add_to_gguf(self.gguf_writer)

def set_gguf_parameters(self):
Expand Down
2 changes: 2 additions & 0 deletions src/llama-chat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ llm_chat_template llm_chat_detect_template(const std::string & tmpl) {
}
} else if (tmpl_contains("<|assistant|>") && tmpl_contains("<|end|>")) {
return LLM_CHAT_TEMPLATE_PHI_3;
} else if (tmpl_contains("[gMASK]<sop>") && tmpl_contains("<|assistant|>") && tmpl_contains("<|user|>")) { /* GLM4 0414 models */
return LLM_CHAT_TEMPLATE_CHATGML_4;
} else if (tmpl_contains("<|assistant|>") && tmpl_contains("<|user|>")) {
return tmpl_contains("</s>") ? LLM_CHAT_TEMPLATE_FALCON_3 : LLM_CHAT_TEMPLATE_GLMEDGE;
} else if (tmpl_contains("<|{{ item['role'] }}|>") && tmpl_contains("<|begin_of_image|>")) {
Expand Down
Loading