-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Function calling for OpenAI backend #573
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
Conversation
0b7d7f1 to
fbe44c2
Compare
Ying1123
left a comment
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 for the PR! I left a few comments. The review is still in progress.
python/sglang/lang/interpreter.py
Outdated
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.
use dictionary order instead.
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.
Removing this given we are moving it to be part of SglGen.
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.
We may also want to retrieve the results from function call.
Add tests for state["func_call_1"] in the function single().
python/sglang/backend/openai.py
Outdated
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.
s.messages_ should be updated after function call finished rather than in generate, and the append logic should happen in interpreter.py, see _execute_role_end() as a reference.
Additionally, changes prompt implicitly changes s.messages_. This is not safe. Changes s.messages_ then set prompt = s.messages_ is better.
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.
Restructured the code a little bit based on your suggestions (with some minor tweaks but I can update if you think it's still better to move the function call generate call outside of generate (we will just have a simpler generate call):
Within openai.py
build_function_call_messages(): a new function which builds function call messages. Given function signature is specific to open ai models, keeping the logic to parse inputs and produce function call messages within the backend code.generate(): Given prompt is local to thegenerate()call, I directly addedfunction_call_messagesto it so that we can call with function call messages during the current completion call's prompt. The main intuition is to try resuing the generate call logic and it also only appends function call response (comp) without intermediate messages into the final text/messages.
Within interpreter.py
- Updated
_execute_gen()logic to include building function call messages if tools are provided, and handle both parallel function calling and non-parallel function calling by either callingbackend.generateone time for parallel function call supported models, or multiple times if parallel call is not supported.
python/sglang/backend/openai.py
Outdated
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.
keep in mind that the set of models that support function calling and parallel function calling are different.
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 for pointing out! Updated to have different handling logic.
python/sglang/backend/openai.py
Outdated
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 this case, assert tool_choice is in names of candidate functions.
python/sglang/backend/openai.py
Outdated
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 it is better to put the logic of real function call into the interpreter, so that it can be reused when we develop the feature for local models.
And remember to handle the logic of appending s.messages_ and s.text_ in the interpreter.
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.
Makes sense, returning just the function call messages here so we can do real function call separately.
python/sglang/lang/interpreter.py
Outdated
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 yes, by default. Although accumulating functions could be an option.
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.
A design suggestion is that it might be better to just have sgl.gen with func_call as an argument.
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 agree, I think it's more straightforward to have it as part of sgl.gen. Would it make sense to have something like sgl.gen("answer_1", max_tokens=256, sgl.func_call(...)) or simply expose parameters directly to sgl.gen like sgl.gen("answer_1", max_tokens=256, tools=[...])?
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.
Let's simply expose parameters directly to sgl.gen.
e859d3e to
edc30d2
Compare
2463404 to
d737da5
Compare
5b3918a to
075b053
Compare
7eda0c8 to
41d1f67
Compare
e1cbcf5 to
2a01552
Compare
f58f983 to
1e978c2
Compare
16b1dbf to
f1389dc
Compare
|
Any updates on this? :) |
|
It is now supported #2544 |
|
Close this due to inactivity and #2544. |
Adding skeleton code for function calling with Open API models.
Example output (when
tool_choice is "auto" or "required"):Example output (when
tool_choice is "none"):The current implementation does not support: