Skip to content

Add support for OpenAI API#443

Closed
julien-blanchon wants to merge 9 commits intohuggingface:mainfrom
julien-blanchon:add-chatgpt-api
Closed

Add support for OpenAI API#443
julien-blanchon wants to merge 9 commits intohuggingface:mainfrom
julien-blanchon:add-chatgpt-api

Conversation

@julien-blanchon
Copy link
Copy Markdown
Contributor

@julien-blanchon julien-blanchon commented Sep 17, 2023

This PR add the support for the OpenAI API

This implementation add a new route /openai/ that wrap the OpenAI API in a HuggingFace Inferencer fashion.
This route return a StreamingResponse if the stream parameter is set to true.
I did try to mimic the HuggingFace Inferencer as much as possible.

To call internal /openai/ route in generateFromDefaultEndpoint I added the sveltekit fetch in argument.
Maybe it's not the best way to do it ...

I did a little update of the README.md to add some explanations about the OpenAI API.

Screen.Recording.2023-09-17.at.23.38.11.mov

TODO:

  • Fix the max_new_tokens issue
  • Improve the README.md documentation
  • Improve the /openai route to be more generic

@julien-blanchon
Copy link
Copy Markdown
Contributor Author

julien-blanchon commented Sep 18, 2023

To avoid adding dependencies I didn't add the openai package but nice idea could be to add the langchain.js package and to adapt my script for any hosted service that langchain support (Claude, Palm, Vertex, ...) and will support in the future.

What is your thoughts about this ?

@Maemol
Copy link
Copy Markdown

Maemol commented Sep 18, 2023

I think it's a good idea to support langchain.js. I was working on implementing a similar solution with the @azure/openai package but it would make sense to use langchain instead. I'm going to look at your implementation tonight. Thank you!

@julien-blanchon
Copy link
Copy Markdown
Contributor Author

julien-blanchon commented Sep 19, 2023

About langchain.js:

We should wait for feedback from the HuggingFace team before using langchain.js. Langchain has features like the SerpAPI web search, but HuggingFace is also making something similar called Agents.js. We don't want any clashes and need to rewrite in the future. So, it's best to add langchain.js if we're sure it's beneficial. I think it's a good idea to hear from an expert about this.


Updates on the Current PR:

I found a bug. When we change the max_new_tokens, there's an error. It seems like something's off with the /openai endpoint. I've marked this for future fixing.

Also, I want to make the /openai route more flexible. We might use it again with other tools.

About the token format: I've suggested a layout to make our work with the OpenAI API easier:

  "userMessageToken": "",
  "assistantMessageToken": "",
  "userMessageEndToken": "</s>",
  "assistantMessageEndToken": "</s>",
  "preprompt": "You are a helpful assistant...</s>",

Is this format okay for other tools we might use later?

@helterskelterr
Copy link
Copy Markdown

really good change, would this also work with the new gpt-3.5 instruct model that just got released

@julien-blanchon
Copy link
Copy Markdown
Contributor Author

gpt-3.5 instruct

Yes of course every OpenAI model work (include gpt-3.5-turbo-instruct), you just need to change the model name in the .env config. The only OpenAI features that don't work is function

@chenhunghan
Copy link
Copy Markdown
Contributor

Looks like we have similar ideas :) #452

@julien-blanchon
Copy link
Copy Markdown
Contributor Author

Looks like we have similar ideas :) #452

Nice 🤗

I like your PR, especially the openAIChatToTextGenerationStream, it's a the same mecanism but with a quite better style.
I think I will close this PR and we will continue to work there.

See you in #452 🙂

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