dspy icon indicating copy to clipboard operation
dspy copied to clipboard

Published 20 hours ago • verified publisher icon stanfordnlp

fix(dspy): together client response parsing bug + endpoint

Open adam-simple opened this issue 5 months ago • 10 comments

I opened up a related issue just now https://github.com/stanfordnlp/dspy/issues/626

Basically I was getting errors when using the Together client. If I set use_chat_api to true, the response did not have an "output" field. Also, if I set use_chat_api to false, I get a 404. These changes fix these issues.

I've only done a smoke test. probably worth at least a smoke test by whoever is reviewing this as well.

adam-simple avatar Mar 11 '24 02:03 adam-simple

Thanks @adam-simple ! LGTM, but @insop do you want to run some checks on this? (since you added the Together client :).

arnavsinghvi11 avatar Mar 11 '24 22:03 arnavsinghvi11

Thank you @arnavsinghvi11 for looping me.

@adam-simple Thank you for the PR.

I was using this API base as follows, and when you use this API_BASE, it was correct in both API URL and the output.

TOGETHER_API_BASE="https://api.together.xyz/inference"`

But, when I check the API doc, https://api.together.xyz/inference is deprecated.

So your change is great to remove the deprecated url.

If you could consider the follow two, it will be great.

  1. could you keep url as is url = f"{self.api_base}", so that TOGETHER_API_BASE is set to either https://api.together.xyz/v1/chat/completions or https://api.together.xyz/v1/completions
  2. if you could add API doc and the above TOGETHER_API_BASE to this document at docs/api/language_model_clients/Together.md it will be help others very much.

insop avatar Mar 12 '24 02:03 insop

@adam-simple it's not working I've tried the fix you mentioned can you please inform what did you set for TOGETHER_API_BASE

someshfengde avatar Mar 19 '24 06:03 someshfengde

by using api_base as https://api.together.xyz/v1/chat image

by using api_base as https://api.together.xyz/v1 image

Also tried by using use_chat_api = True

image

something is wrong with it ig

someshfengde avatar Mar 19 '24 06:03 someshfengde

@someshfengde

Please seem y post above and that's what I was suggesting.

insop avatar Mar 19 '24 07:03 insop

I resolved this issue by using the OpenAI compatability together has. I created another handler for together.

using existing one was too much hassle for me.

If anyone needed here is the branch for fix:

https://github.com/curieo-org/dspy/tree/togetherAPIFIX

someshfengde avatar Mar 19 '24 08:03 someshfengde

Oops I spoke too fast. OK it seems that this is good to go once the issue faced by @someshfengde is fixed

okhat avatar Mar 19 '24 13:03 okhat

Hi @someshfengde , did you want to add your changes for Together to this PR? maybe the ones that were removed from the other PR?

arnavsinghvi11 avatar Apr 02 '24 19:04 arnavsinghvi11

Hi @someshfengde , did you want to add your changes for Together to this PR? maybe the ones that were removed from the other PR?

I'd have to open a different PR ig? right?

someshfengde avatar Apr 03 '24 05:04 someshfengde

@adam-simple @someshfengde should this PR be closed?

arnavsinghvi11 avatar Apr 13 '24 01:04 arnavsinghvi11