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: integrate the llama3 (8B, 70B) served by Groq #531
base: master
Are you sure you want to change the base?
Conversation
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@Wendong-Fan Hi, could you please help me add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Left a question
camel/models/groq_model.py
Outdated
if not self._token_counter: | ||
# Groq API does not provide any token counter, so we use the | ||
# OpenAI token counter as a placeholder. | ||
self._token_counter = OpenAITokenCounter(ModelType.GPT_3_5_TURBO) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use Autotokenizer
to load the correct llama3 tokenizer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, and I will try it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I have implemented the Autotokenizer. wait for your review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Appointat 's contribution! Overall is great, left some comments
class GroqLlama3TokenCounter(BaseTokenCounter): | ||
def __init__(self, model_type: ModelType): | ||
r"""Constructor for the token counter for Llama3 models served by Groq. | ||
|
||
Args: | ||
model_type (ModelType): Model type for which tokens will be | ||
counted. | ||
""" | ||
|
||
self.model_type = model_type | ||
# Since Groq API does not provide any token counter, we use the | ||
# AutoTokenizer from transformers as a placeholder. | ||
self.tokenizer = AutoTokenizer.from_pretrained('bert-base-uncased') | ||
|
||
def count_tokens_from_messages(self, messages: List[OpenAIMessage]) -> int: | ||
r"""Count number of tokens in the provided message list using | ||
loaded tokenizer specific for this type of model. | ||
|
||
Args: | ||
messages (List[OpenAIMessage]): Message list with the chat history | ||
in Groq llama3 API format. | ||
|
||
Returns: | ||
int: Number of tokens in the messages. | ||
""" | ||
num_tokens = 0 | ||
for message in messages: | ||
for _, value in message.items(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use class OpenSourceTokenCounter
do this?
additional arguments to check. | ||
Raises: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
additional arguments to check. | |
Raises: | |
additional arguments to check. | |
Raises: |
_choices: List[Choice] = [] | ||
for choice in response.choices: | ||
choice.message = ChatCompletionMessage( | ||
role=choice.message.role, # type: ignore[arg-type] | ||
content=choice.message.content, | ||
tool_calls=choice.message.tool_calls, # type: ignore[arg-type] | ||
) # type: ignore[assignment] | ||
_choice = Choice(**choice.__dict__) | ||
_choices.append(_choice) | ||
response.choices = _choices # type: ignore[assignment] | ||
|
||
response.usage = CompletionUsage(**response.usage.__dict__) # type: ignore[assignment] | ||
return ChatCompletion(**response.__dict__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused with why we use Choice
here, the response is already ChatCompletion
. could you explain a little bit? thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some comments
self.model_type = model_type | ||
# Since Groq API does not provide any token counter, we use the | ||
# AutoTokenizer from transformers as a placeholder. | ||
self.tokenizer = AutoTokenizer.from_pretrained('bert-base-uncased') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest not launching any tokenizer if allowed. User won't go through every line of code but just find there is a tokenizer and use it. They can not notice it is not the correct one until they notice some errors in token number and try to figure out what's the problem.
Or, we need to specify which open-source model is launched and try to load it via HF libraries.
Description
Describe your changes in detail.
Motivation and Context
Types of changes
What types of changes does your code introduce? Put an
x
in all the boxes that apply: