Skip to content
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.

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

feat(glm3): adapt to glm3 prompt #2620

Closed
wants to merge 4 commits into from
Closed

Conversation

silk55
Copy link

@silk55 silk55 commented Oct 31, 2023

@infwinston
Copy link
Member

Thanks @silk55 for the contribution! We'd love to add support for ChatGLM3.

@lucasjinreal
Copy link

LGTM.

@lucasjinreal
Copy link

Oh, I think system message after should add a \n

@ZeyuTeng96
Copy link
Contributor

hi there,

check this: #2622

I think I follow the format
@infwinston @lucasjinreal @silk55

@@ -140,6 +140,17 @@ def get_prompt(self) -> str:
elif self.sep_style == SeparatorStyle.CHATGLM:
# source: https://huggingface.co/THUDM/chatglm-6b/blob/1d240ba371910e9282298d4592532d7f0f3e9f3e/modeling_chatglm.py#L1302-L1308
# source2: https://huggingface.co/THUDM/chatglm2-6b/blob/e186c891cf64310ac66ef10a87e6635fa6c2a579/modeling_chatglm.py#L926
if self.name == "chatglm3":
Copy link
Member

Choose a reason for hiding this comment

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

Could you put a link to the source here?

Copy link
Contributor

@ZeyuTeng96 ZeyuTeng96 Nov 1, 2023

Choose a reason for hiding this comment

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

Hi there,

since chatglm3 uses differenet chat format, I do not think we still need to use the 'CHATGLM' separator style and put a if condition to choice the chat format. But putting the chatglm3 chat format into 'CHATGLM' separator style make sense. I suggest an main contributer to dicide do we have to make an new separator style for it @merrymercy

Copy link
Member

Choose a reason for hiding this comment

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

@silk55 could you add a source?

@merrymercy merrymercy mentioned this pull request Oct 31, 2023
@merrymercy
Copy link
Member

Hi @lucasjinreal @yanyang1024 @silk55 @ZeyuTeng96. You all added the ChatGLM-3 support (#2618, #2620, #2622).
Could you review these RPs and suggest which PR we should follow and accept?

@merrymercy
Copy link
Member

closed by #2622

@merrymercy merrymercy closed this Nov 10, 2023
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.

5 participants