-
Notifications
You must be signed in to change notification settings - Fork 87
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
Add the support for GPT-4o model #398
base: main
Are you sure you want to change the base?
Conversation
llm_toolkit/models.py
Outdated
@@ -198,7 +198,10 @@ def estimate_token_num(self, text) -> int: | |||
"""Estimates the number of tokens in |text|.""" | |||
# https://cookbook.openai.com/examples/how_to_count_tokens_with_tiktoken | |||
try: | |||
encoder = tiktoken.encoding_for_model(self.name) | |||
if 'gpt-4' in self.name: # gpt-4 and gpt-4o | |||
encoder = tiktoken.encoding_for_model('gpt-4') |
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.
What is the reason for not specifying gpt-4o
directly? https://github.com/openai/tiktoken/blob/c0ba74c238d18b4824c25f3c27fc8698055b9a76/tiktoken/model.py#L22-L23
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, @DavidKorczynski. I see the link you provided claimed the support for gpt-4o
. But it seems now it gets the error below if running the code encoder = tiktoken.encoding_for_model(self.name)
for gpt-4o
directly:
Python 3.11.7 (main, Dec 8 2023, 18:56:57) [GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import tiktoken
>>> encoder = tiktoken.encoding_for_model("gpt-4o")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/kaixuan/FDG_LLM/oss-fuzz-gen/.venv/lib/python3.11/site-packages/tiktoken/model.py", line 97, in encoding_for_model
return get_encoding(encoding_name_for_model(model_name))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/kaixuan/FDG_LLM/oss-fuzz-gen/.venv/lib/python3.11/site-packages/tiktoken/model.py", line 84, in encoding_name_for_model
raise KeyError(
KeyError: 'Could not automatically map gpt-4o to a tokeniser. Please use `tiktoken.get_encoding` to explicitly get the tokeniser you expect.'
you can confirm it again on your env.
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.
Likely because of
Line 14 in 3d20914
tiktoken==0.5.1 |
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.
Ok~
I have tried tiktoken 0.7.0
, it works now if directly running encoder = tiktoken.encoding_name_for_model("gpt-4o")
after upgrading the tiktoken.
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.
@DavidKorczynski Do I need to recreate a PR to directly use tiktoken
to get the encoding for gpt-4o? Or we can continue use this PR (if you can revise the code)?
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 think what we need is update tiktoken
to a newer version that includes gpt-4o
and remove the changes in estimate_token_num
as we no longer need them -- you can just do that in this PR and that should make a complete contribution in terms of gpt-4o
support
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.
Got it. I have finished and upgraded all dependencies needed by following https://github.com/google/oss-fuzz-gen/blob/main/USAGE.md#updating-dependencies as well.
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 @MarkLee131 -- if #433 is happy let's land this
@@ -1,5 +1,5 @@ | |||
# | |||
# This file is autogenerated by pip-compile with Python 3.12 | |||
# This file is autogenerated by pip-compile with Python 3.11 |
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.
Are you sure changing the version doesn't cause regressions?
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.
In addition, do we really need to update all the libs for a tiktoken update?
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've actually been running oss-fuzz-gen with python 3.11 and it should be fine.
Meanwhile, since some packages may be dependent on tiktoken, for insurance purposes, I just tried to update all the dependencies directly. :)
gpt-4o
(https://platform.openai.com/docs/models/gpt-4o) and