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

update openai wrapper to work with tiktoken interface and newest openai version #794

Merged
merged 22 commits into from
Dec 14, 2023

Conversation

bmosaicml
Copy link
Contributor

@bmosaicml bmosaicml commented Dec 9, 2023

Some changes to the OpenAI tokenizer (tiktoken) made the previously working e2e OpenAI eval wrapper no longer work with the eval script.

Some changes to the newest version of OpenAI API include deprecating the openai.ChatCompletion.create method in favor of first constructing a client and calling client.chat.completion.create. Additionally, return results are no longer dictionaries , but instead openai.types.Completion objects

Tested with composer eval/eval.py eval/yamls/openai_eval.yaml

Ran on all of lm_tasks_v0.2: openai-eval-O5OK4O

dakinggg
dakinggg previously approved these changes Dec 11, 2023
Copy link
Collaborator

@dakinggg dakinggg left a comment

Choose a reason for hiding this comment

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

Thank you! Could you please add a note to the yaml noting which tasks are supported with which models? (some are not supported because we can't get logits, right?)

@bmosaicml bmosaicml requested a review from a team as a code owner December 11, 2023 23:02
@bmosaicml bmosaicml changed the title update openai wrapper to work with tiktoken interface update openai wrapper to work with tiktoken interface and newest openai version Dec 11, 2023
@bmosaicml bmosaicml enabled auto-merge (squash) December 14, 2023 17:52
@maxisawesome
Copy link
Contributor

with the changes, LGTM 👍

Copy link
Collaborator

@dakinggg dakinggg left a comment

Choose a reason for hiding this comment

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

Approving to unblock the setup.py change. Deferring the full review to Max.

@bmosaicml bmosaicml merged commit 5cc4dd4 into main Dec 14, 2023
8 checks passed
@dakinggg dakinggg deleted the update_openai_wrapper branch February 3, 2024 01:27
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.

4 participants