-
Notifications
You must be signed in to change notification settings - Fork 13.4k
common : fix reasoning before forced tool call via tool_choice = required #16264
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
common : fix reasoning before forced tool call via tool_choice = required #16264
Conversation
|
Thanks for this! I'll check it more thoroughly this weekend but here are my initial thoughts:
|
|
What do you think about this aldehir@c746984? I wrote a simple script that provided a calculator to First, I used Multi-turn scenario with tool_choice=autoTurn 1MessageCalculate the following expressions one by one using the calculator tool.
ResponseTurn 2Tool Call Result14.0 ResponseTurn 3Tool Call Result-4.0 ResponseTurn 4Tool Call Result1.0 ResponseTurn 5Tool Call Result6.0 ResponseTurn 6Tool Call Result12.0 ResponseTurn 7Tool Call Result16.0 ResponseTurn 8Tool Call Result7.0 ResponseTurn 9Tool Call Result0.9999999999999999 ResponseTurn 10Tool Call Result99.0 ResponseTurn 11Tool Call Result9.0 ResponseFrom this, you can see a couple of things:
Here is an example using the commit I linked. Multi-turn scenario with tool_choice=requiredTurn 1MessageCalculate the following expressions one by one using the calculator tool.
ResponseTurn 2Tool Call Result14.0 ResponseTurn 3Tool Call Result-4.0 ResponseTurn 4Tool Call Result1.0 ResponseTurn 5Tool Call Result6.0 ResponseTurn 6Tool Call Result12.0 ResponseTurn 7Tool Call Result16.0 ResponseTurn 8Tool Call Result7.0 ResponseTurn 9Tool Call Result0.9999999999999999 ResponseTurn 10Tool Call Result99.0 ResponseTurn 11Tool Call Result9.0 ResponseNote: I did set tool_choice=auto on the last turn just so the model produces a final message. Using the commit above, Let me know if this handles your use cases, or if there's anything I am missing. |
|
Thanks for the quick reply! I built and ran your commit and it works for my use case as well. I saw the other case for tool calls in the commentary channel, but I wasn't sure what to do there. And I run 20b anyway. But your solution is much better |
Awesome. Feel free to cherry-pick the commit to your PR. |
…s required (cherry picked from commit c746984)
…ired (ggml-org#16264) * common : fix reasoning before forced tool call via tool_choice = required * common : improve reasoning and commentary handling when tool_choice is required (cherry picked from commit c746984) --------- Co-authored-by: Alde Rojas <[email protected]>
…ired (ggml-org#16264) * common : fix reasoning before forced tool call via tool_choice = required * common : improve reasoning and commentary handling when tool_choice is required (cherry picked from commit c746984) --------- Co-authored-by: Alde Rojas <[email protected]>
Make sure to read the contributing guidelines before submitting a PR
Hello! This patch allows gpt-oss models to reason before doing a tool call, when
tool_choiceis set to required.A similar issue, but for Qwen3/Hermes 2: #15247