Skip to content

Conversation

@kylrth
Copy link
Contributor

@kylrth kylrth commented Jun 15, 2023

Trying to call ChatOpenAI.get_num_tokens_from_messages returns the following error for the newly announced models gpt-3.5-turbo-0613 and gpt-4-0613:

NotImplementedError: get_num_tokens_from_messages() is not presently implemented for model gpt-3.5-turbo-0613.See https://github.com/openai/openai-python/blob/main/chatml.md for information on how messages are converted to tokens.

This adds support for counting tokens for those models, by counting tokens the same way they're counted for the previous versions of gpt-3.5-turbo and gpt-4.

reviewers

@hwchase17 hwchase17 merged commit c7db9fe into langchain-ai:master Jun 15, 2023
@pors
Copy link
Contributor

pors commented Jun 15, 2023

Maybe also update this and make the newer models the default?

    def _get_encoding_model(self) -> Tuple[str, tiktoken.Encoding]:
        tiktoken_ = _import_tiktoken()
        model = self.model_name
        if model == "gpt-3.5-turbo":
            # gpt-3.5-turbo may change over time.
            # Returning num tokens assuming gpt-3.5-turbo-0301.
            model = "gpt-3.5-turbo-0301"
        elif model == "gpt-4":
            # gpt-4 may change over time.
            # Returning num tokens assuming gpt-4-0314.
            model = "gpt-4-0314"

@kylrth
Copy link
Contributor Author

kylrth commented Jun 15, 2023

@pors why do we modify the model name at all? Shouldn't we rely on tiktoken to choose if the model we're using is just the default unversioned tag? Here's how tiktoken does it: https://github.com/openai/tiktoken/blob/5d970c1100d3210b42497203d6b5c1e30cfda6cb/tiktoken/model.py#L7

@pors
Copy link
Contributor

pors commented Jun 15, 2023

Yeah, I'm not sure why that code is in there to be honest

@kylrth
Copy link
Contributor Author

kylrth commented Jun 15, 2023

@hwchase17 any input on this?

This was referenced Jun 25, 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.

3 participants