Closed michael-newsrx closed 2 weeks ago
If you want to set top_p to 1.0, you can simply sending the top_p as none, which will result in the default value of 1.0 being applied.
It seems like the equal condition in the code is causing an error.
This doesn't provide a resolution to the issue.
The docker container rejects top_p=1.0
for OpenAI interface, but OpenAI interface should accept top_p=1.0
and not fail.
Is there an easy way to "patch" the container and deploy using the patched version?
If you want to set top_p to 1.0, you can simply sending the top_p as none, which will result in the default value of 1.0 being applied.
It seems like the equal condition in the code is causing an error.
See also: https://github.com/guidance-ai/guidance/issues/945
Hi @michael-newsrx
Thank you for bringing this to our attention and for making the PR 👍
As far as I can tell, there shouldn't be anything blocking for getting this merged in. I'll approve running the CI and can take over the merging of the PR.
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
@ErikKaum I ran into this issue as I switched out API base urls and suddenly my script broke as the new API uses TGI which doesnt allow top_p=1.0
. I can work around this but it would be nice if TGI would allow this as I dont see why it is not allowed.
Hi @cornzz 👋
I understand that it's annoying that it breaks the client. But I think still for now we're opting for a clear error VS discarding user input without letting the user know.
But if there's a lot of demand for the top_p=1.0 == no top_p
alternative we're still open. One good way to get an "indication of demand" for that would be e.g. to get thumbs up on a issue that's a feature request.
Hopefully this makes sense to you 👍
Depending on the client software. It could result in a breakage that prevents any use of TGI by a customer.
-Michael Conrad Telephone: +1.678.934.3989 Email: @. Telegram: https://t.me/mconrad202 Mastodon: @.
On Tue, Sep 3, 2024 at 9:47 AM Erik Kaunismäki @.***> wrote:
Hi @cornzz https://github.com/cornzz 👋
I understand that it's annoying that it breaks the client. But I think still for now we're opting for a clear error VS discarding user input without letting the user know.
But if there's a lot of demand for the top_p=1.0 == no top_p alternative we're still open. One good way to get an "indication of demand" for that would be e.g. to get thumbs up on a issue that's a feature request.
Hopefully this makes sense to you 👍
— Reply to this email directly, view it on GitHub https://github.com/huggingface/text-generation-inference/issues/2222#issuecomment-2326574074, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABH4XNG7FICQDFNQVTEVASLZUW4WRAVCNFSM6AAAAABKXPZBUWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRWGU3TIMBXGQ . You are receiving this because you commented.Message ID: @.***>
Hey @ErikKaum, thanks for your quick response! Its not a problem, I was reusing a script and I am wondering why the authors set top_p
at all since it defaults to 1.
Still, and sorry if I am misunderstanding something, but what do you mean by discarding user input? Maybe I am missing something, but why can't top_p
be set to 1.0
by the user manually while not setting any value for top_p
makes it default to 1.0
?
System Info
Information
Tasks
Reproduction
Fails
Works
Expected behavior
See also: https://github.com/huggingface/text-generation-inference/issues/1896 where the patch did not address this issue even though raised as part of the ticket.
Impact
This generally breaks libraries like
guidance
where the library is hard coded to usetop_p=1.0
for the OpenAI interface.