Skip to content

Comments

feat: use model name as adapter id in chat endpoints#2128

Merged
Narsil merged 1 commit intomainfrom
enable-adapter-id-in-chat
Jul 8, 2024
Merged

feat: use model name as adapter id in chat endpoints#2128
Narsil merged 1 commit intomainfrom
enable-adapter-id-in-chat

Conversation

@drbh
Copy link
Collaborator

@drbh drbh commented Jun 26, 2024

This PR allows users to specify lora adapter ids as the model value in the /v1/chat/completions and /v1/completions endpoints.

This feature aligns with other lora implementations and was mentioned here during the initial lora PR #2010 (comment)

curl localhost:3000/v1/chat/completions -s \
    -H 'Content-Type: application/json'  \
    -X POST \
    -d '{
  "model": "predibase/customer_support",
  "messages": [
    {
      "role": "system",
      "content": "You are a helpful assistant."
    },
    {
      "role": "user",
      "content": "What is deep learning?"
    }
  ],
  "stream": false,
  "max_tokens": 20, "seed": 42
}' | jq .

top_n_tokens: None,
grammar: None,
..Default::default()
adapter_id: model.as_ref().filter(|m| *m != "tgi").map(String::from),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this special value ? It should exist in the first place.

if openai requires a model id, then we should actually match it against the actual deployed model id, no ?

Copy link
Contributor

@Narsil Narsil left a comment

Choose a reason for hiding this comment

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

Merging.

"tgi" is used throughout our docs. Just for backward compat we should support it somehow.

Definitely not great though.

@Narsil Narsil merged commit 87ebb64 into main Jul 8, 2024
@Narsil Narsil deleted the enable-adapter-id-in-chat branch July 8, 2024 14:06
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