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

fix(#5818): Force to use string serializer for deepseek function calling #5824

Merged
merged 4 commits into from
Dec 26, 2024

Conversation

xingyaoww
Copy link
Collaborator

@xingyaoww xingyaoww commented Dec 26, 2024

End-user friendly description of the problem this fixes or functionality that this introduces

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

Give a summary of what the PR does, explaining any non-trivial design decisions

Force to use string serializer for deepseek function calling, otherwise we will end up with the error in #5818.

Running an evaluation now with Deepseek w/ function calling..

Still use our non-fncall converter for Deepseek - see results here: https://x.com/xingyaow_/status/1872375832041103638


Link of any specific issues this addresses


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:08860cd-nikolaik   --name openhands-app-08860cd   docker.all-hands.dev/all-hands-ai/openhands:08860cd

@xingyaoww xingyaoww requested a review from enyst December 26, 2024 16:58
Copy link
Collaborator

@enyst enyst left a comment

Choose a reason for hiding this comment

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

Looking good to me!

I knew this moment will come, when the string/list choice will not depend only on our attributes...😅 I think this might be something to fix in litellm if indeed the list format is sent but not supported for deepseek.

Just left a comment to keep our changes minimal.

openhands/llm/llm.py Outdated Show resolved Hide resolved
@xingyaoww xingyaoww marked this pull request as ready for review December 26, 2024 20:30
@xingyaoww xingyaoww enabled auto-merge (squash) December 26, 2024 20:30
@xingyaoww xingyaoww merged commit a021045 into main Dec 26, 2024
12 checks passed
@xingyaoww xingyaoww deleted the xw/fix-dpsk branch December 26, 2024 20:45
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.

2 participants