Skip to content

Support tool use and add example #174

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

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

DePasqualeOrg
Copy link
Contributor

@DePasqualeOrg DePasqualeOrg commented Jan 1, 2025

This demonstrates tool use (function calling), which is now supported in my PR to Swift Jinja. You should make sure that you have the latest tokenizer_config.json file for each model, since in some cases function calling was added in a recent update.

The following are some examples of responses to the prompt What's the weather in Paris today? in LLMEval. A get_current_weather function is provided to the model in the prompt constructed with the chat template.

Llama 3.1 8B

<|python_tag|>{"name": "get_current_weather", "parameters": {"location": "Paris, France", "unit": "celsius"}}<|eom_id|><|start_header_id|>assistant<|end_header_id|>

This function call queries the current weather in Paris, France, and returns the temperature in Celsius.

Llama 3.2 3B

<|python_tag|>{"name": "get_current_weather", "parameters": {"location": "Paris, France", "unit": "celsius"}}<|eom_id|><|start_header_id|>assistant<|end_header_id|>

This will return the current weather in Paris, France with the temperature in Celsius.

Qwen 2.5 7B

<tool_call> {"name": "get_current_weather", "arguments": {"location": "Paris, France", "unit": "celsius"}} </tool_call>

Mistral 7B

[TOOL_CALLS] [{"name": "get_current_weather", "arguments": {"location": "Paris, France"}}]

Performance

Llama 3.1 8B and 3.2 3B with the current chat templates from tokenizer_config.json tend to always respond with a function call, even when not appropriate. My proposed change to the chat template helps, but the models still sometimes respond with calls to non-existent functions. In general, the prompts provided in the Llama chat templates are far from optimal, and I think the models' performance could be further improved simply by using better prompts (for example, "Knowledge cutoff date" instead of "Cutting Knowledge Date").

Qwen 2.5 7B and Mistral 7B do a better job of calling functions only when appropriate.

Handling the function call

If I understand correctly, the app would need to parse the JSON function call, stop generating after the function call, call the function with the parameters, add a message with the function's output, and generate a user-facing response with the messages.

@davidkoski
Copy link
Collaborator

This looks really interesting!

@DePasqualeOrg
Copy link
Contributor Author

This is almost ready, but some more changes in swift-transformers might be necessary: huggingface/swift-transformers#169

@DePasqualeOrg DePasqualeOrg force-pushed the tool-use-example branch 3 times, most recently from aa1afcc to 559a44c Compare January 31, 2025 16:19
@DePasqualeOrg DePasqualeOrg changed the title Tool use example Support tool use and add example Jan 31, 2025
@DePasqualeOrg
Copy link
Contributor Author

This is now ready for review. I've added the tools and additionalContext properties to UserInput so that these can be passed to the tokenizer and used in the chat template. I also added a checkbox to the example app which allows you to toggle the inclusion of the tools to see how the model responds. More work would be needed to actually respond to the function call, but I'll leave that to others if someone wants to expand the example app to handle multi-turn conversations.

@DePasqualeOrg DePasqualeOrg marked this pull request as ready for review January 31, 2025 16:57
@@ -126,8 +130,6 @@ struct ContentView: View {

}
.task {
self.prompt = llm.modelConfiguration.defaultPrompt
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be put back once the prompt override is removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Above you had hard coded the prompt for development -- once that part is reverted this should go back in (the per model prompt)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I've reverted that part.

Toggle(isOn: $llm.includeWeatherTool) {
Text("Include \"get current weather\" tool")
}
.frame(maxWidth: 350, alignment: .leading)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tool enabled:

image

and disabled:

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is a great way to see the tool integration in action. I wonder if it should be included in this generic UI? I guess it is an example and people who use this as the basis for their own app can either include it or not, depending on their needs.

// let modelConfiguration = ModelRegistry.llama3_1_8B_4bit
// let modelConfiguration = ModelRegistry.mistral7B4bit
// let modelConfiguration = ModelRegistry.qwen2_5_7b
let modelConfiguration = ModelRegistry.qwen2_5_1_5b
Copy link
Collaborator

Choose a reason for hiding this comment

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

Changing this to qwen is fine (it comes in at 1.7G) but please update the comment and remove the commented out parts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"required": ["location"],
] as [String: any Sendable],
] as [String: any Sendable],
] as [String: any Sendable]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is pretty unwieldy :-) I see it matches the type in Transformers -- it is too bad we don't have a struct that could be used to declare this and convert to that type. Perhaps an exercise for future refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at using a struct, but it seems even more unwieldy, although it would be more type-safe. Perhaps someone else can find a more elegant solution than me.

static public let qwen205b4bit = ModelConfiguration(
id: "mlx-community/Qwen1.5-0.5B-Chat-4bit",
overrideTokenizer: "PreTrainedTokenizer",
defaultPrompt: "why is the sky blue?"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be removed? It is older, but people might still refer to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added it back, although I think it's fine to remove outdated models, since the pattern is clear from the existing examples.

@davidkoski
Copy link
Collaborator

@DePasqualeOrg this looks really cool! I added a few comments on removing some of the debug/dev code. It looks like there is a conflict in UserInput.swift now (I think from the merge of the video processing for VLMs).

@DePasqualeOrg DePasqualeOrg force-pushed the tool-use-example branch 3 times, most recently from 4c64908 to a4d206e Compare February 4, 2025 22:46
@DePasqualeOrg
Copy link
Contributor Author

Thanks! I resolved the merge conflict and made the changes you suggested.

Copy link
Collaborator

@davidkoski davidkoski left a comment

Choose a reason for hiding this comment

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

Awesome work, thank you!

@davidkoski davidkoski merged commit 983eaac into ml-explore:main Feb 5, 2025
3 checks passed
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