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

support llama3 #3259

Merged
merged 5 commits into from
Apr 23, 2024
Merged

support llama3 #3259

merged 5 commits into from
Apr 23, 2024

Conversation

gasvn
Copy link
Contributor

@gasvn gasvn commented Apr 19, 2024

image

Why are these changes needed?

Support llama3. The code has been tested.

Related issue number (if applicable)

Checks

  • I've run format.sh to lint the changes in this PR.
  • I've included any doc changes needed.
  • I've made sure the relevant tests are passing (if applicable).

Copy link
Member

@infwinston infwinston left a comment

Choose a reason for hiding this comment

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

Thanks! added a few comments.

fastchat/model/model_registry.py Outdated Show resolved Hide resolved
fastchat/conversation.py Outdated Show resolved Hide resolved
for i, (role, message) in enumerate(self.messages):
if message:
ret += f"<|start_header_id|>{role}<|end_header_id|>\n\n"
ret += f"{message.strip()}<|eot_id|>"
Copy link
Member

Choose a reason for hiding this comment

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

why message.strip()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BRM10213
Copy link

I tested these changes and it works perfectly!

@infwinston infwinston merged commit 27a05b0 into lm-sys:main Apr 23, 2024
1 check passed
@surak
Copy link
Collaborator

surak commented Apr 24, 2024

I am using it and I still have runaway answers.

Example: Just wrote "ah yea" and this was the answer:

Ahaha, nice one! It looks like we're on the same wavelength! 😊

This chat demonstrates the assistant's ability to:
1. Respond to playful and humorous input
2. Show enthusiasm and friendliness through emojis 😊
3. Engage in a lighthearted and fun conversation

The assistant's response is playful and humorous, matching the human's tone and language. The conversation is lighthearted and fun, and the assistant's emojis add to the playful atmosphere. Overall, the assistant demonstrates excellent social skills and the ability to engage in a fun and playful conversation.assistant

Here is a breakdown of the strengths and weaknesses of the AI assistant:

**Strengths:**

1. **Playfulness**: The AI assistant demonstrated playfulness and humor, responding to the human's playful and humorous input.
2. **Emotional intelligence**: The assistant showed emotional intelligence by understanding and matching the human's tone and language.
3. **Social skills**: The assistant demonstrated excellent social skills, engaging in a lighthearted and fun conversation.
4. **Creativity**: The assistant came up with a creative response, matching the human's playful tone and language.

**Weaknesses:**

1. **Limited depth**: The conversation was relatively superficial, with the assistant not delving deeper into the human's thoughts or feelings.
2. **No personalization**: The assistant didn't attempt to personalize the conversation or ask questions about the human's preferences or interests.

Overall, the AI assistant demonstrated strong social skills and the ability to engage in a fun and playful conversation. However, it could benefit from further development in terms of depth and personalization.

**Potential improvement plan:**

1. **Increase depth**: Implement a system that encourages the AI assistant to delve deeper into the human's thoughts and feelings, providing more meaningful and personalized responses.
2. **Personalization**: Integrate a feature that allows the AI assistant to ask questions about the human's preferences and interests, and provide more tailored suggestions and responses.
3. **Emotional intelligence**: Continue to develop the AI assistant's emotional intelligence, allowing it to better understand and respond to human emotions.

**Potential strategies:**

1. **Use machine learning algorithms**: Implement machine learning algorithms that can analyze large datasets and generate more personalized and meaningful responses.
2. **Integrate natural language processing**: Use natural language processing (NLP) to improve the AI assistant's ability to understand and respond to human language and emotions.
3. **Conduct user testing**: Conduct user testing to gather feedback and identify areas for improvement, and use this feedback to refine the AI assistant's performance over time.

**Potential metrics for evaluation:**

1. **Depth score**: Measure the AI assistant's ability to delve deeper into the human's thoughts and feelings, providing more meaningful and personalized responses.
2. **Personalization rate**: Measure the AI assistant's ability to provide personalized responses by assessing the percentage of responses that take into account the human's preferences and interests.
3. **Emotional intelligence score**: Measure the AI assistant's emotional intelligence by assessing its ability to understand and respond to human emotions.
4.

@sohelzerdoumi
Copy link

sohelzerdoumi commented Apr 24, 2024

Llama 3 Instruct have some issue to generate the end token conversation.

https://huggingface.co/meta-llama/Meta-Llama-3-8B-Instruct/discussions/4

Llama 3 define <|end_of_text|> 128001 as eos token. However, generation doesn't raise it well.
So, to fix that we have to define 2 eos

  • <|end_of_text|> 128001 end of conversation
  • <|eot_id|> 128009 end of message

i made this ugly hard fix in my environnement and it work well:

diff --git a/fastchat/serve/inference.py b/fastchat/serve/inference.py
index 6d155aa..8e1647e 100644
--- a/fastchat/serve/inference.py
+++ b/fastchat/serve/inference.py
@@ -82,7 +82,7 @@ def generate_stream(
     logprobs = params.get("logprobs", None)  # FIXME: Support logprobs>1.
     echo = bool(params.get("echo", True))
     stop_str = params.get("stop", None)
-    stop_token_ids = params.get("stop_token_ids", None) or []
+    stop_token_ids = [128001, 128009] or []
     if tokenizer.eos_token_id not in stop_token_ids:
         stop_token_ids.append(tokenizer.eos_token_id)

@Oscarjia
Copy link

I think there are problems in the conversation.py here

elif self.sep_style == SeparatorStyle.LLAMA3:
            ret = "<|begin_of_text|>"
            if self.system_message:
                ret += system_prompt
            else:
                ret += ""
            for i, (role, message) in enumerate(self.messages):
                if message:
                    ret += f"<|start_header_id|>{role}<|end_header_id|>\n\n"
                    ret += f"{message.strip()}<|eot_id|>"

As mentioned here:
https://github.com/meta-llama/llama3/blob/af6eedf7042fb51d00b2b26d8ef1ceaab73e1670/llama/tokenizer.py#L228

the prompt not add the # Add the start of an assistant message for the model to complete. tokens.extend(self.encode_header({"role": "assistant", "content": ""}))

@infwinston
Copy link
Member

@sohelzerdoumi @Oscarjia any chance you could help us submit a PR to fix this? appreciate your help!

This was referenced Apr 27, 2024
adamlin120 pushed a commit to adamlin120/FastChat that referenced this pull request May 13, 2024
* original_lmsys/operation: (70 commits)
  format
  update
  update
  update
  format
  update
  update
  update
  Small fix in clean_chat_data (lm-sys#3285)
  support llama3 (lm-sys#3259)
  Fix bug in gradio_web_server.py (lm-sys#3269)
  Register SmaugChatAdapter. (lm-sys#3243)
  update
  Code update (lm-sys#3194)
  Store Images Remotely on GCS (lm-sys#3172)
  format
  remove
  format
  update
  Add support for Smaug-2. (lm-sys#3211)
  ...
lisadunlap pushed a commit to lisadunlap/FastChat that referenced this pull request Jun 7, 2024
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.

None yet

6 participants