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

Support for config_sentence_transformers.json #244

Closed
sam-ulrich1 opened this issue Apr 18, 2024 · 9 comments · Fixed by #312
Closed

Support for config_sentence_transformers.json #244

sam-ulrich1 opened this issue Apr 18, 2024 · 9 comments · Fixed by #312

Comments

@sam-ulrich1
Copy link

Feature request

Add cli option to auto-format input text with config_sentence_transformers.json prompt settings (if provided) before toknizing.

Motivation

A lot of models now expect a prompt prefix so enabling the server-side handle of this allows clients to become model "model-agnostic". We have trouble changing between models since we must support the custom prompt for each specific model on the client side. Server side via config would remove this entriely.

Your contribution

Happy to do the PR myself just want to make sure this would be a welcome contribution

@OlivierDehaene
Copy link
Member

OlivierDehaene commented Jun 17, 2024

Good idea. This could be added to the payload of /embed.
Do you have an example of such a model I could test this feature with?

@sam-ulrich1
Copy link
Author

I'll have to back track what I was working on at the time I made this but yes. This was derived from a need so I just have to go find the model. I'm still happy to offer a PR just wanted to make sure it was welcome before putting the work in

@sam-ulrich1
Copy link
Author

@OlivierDehaene
Copy link
Member

OlivierDehaene commented Jun 20, 2024

So you want to be able to select the prompt with a payload like:

{
  "inputs": "text",
  "prompt": "query"
  "truncate": false
}

And the prompt is just added at the beginning of inputs right?

@sam-ulrich1
Copy link
Author

My preference order would be

  1. Option for the router to load the config and auto format the prompt. Issue here being some prompts are conditional so there would have to be some query param that instructs to format or not. Benefit being that the end application does not have to manage any formatting.
  2. End application passes the prompt format like you described above with some predefined formatting structure. Issue here is that end applications must be aware which creates a "model-lock-in" situation that makes it hard to change embedding models.

Basically we find it a bit limiting to have to change code across multiple codebases when we change models

@OlivierDehaene
Copy link
Member

Ok so what you would like is to have a default format set when starting the service? Otherwise I don't see how you can make it truly model agnostic.
All models can chose whatever format and name they want for this parameter and you need to be able to specifiy it when embedding.

So:

  1. parse the config
  2. set auto format from parameter in cli
  3. embed with default format or specified format in the body

@sam-ulrich1
Copy link
Author

Yes. The main goal would be to reduce friction between model changes. If y'all are comfortable with this, I'd be happy to offer a PR. It will be a few weeks with the holiday and all.

@OlivierDehaene
Copy link
Member

OlivierDehaene commented Jun 27, 2024

The main goal would be to reduce friction between model changes

I don't know how much it reduces friction because the models don't need to agree on the names.
For example:

Snowflake/snowflake-arctic-embed-l

"prompts": {
    "query": "Represent this sentence for searching relevant passages: "
  }

vs

intfloat/e5-mistral-7b-instruct

"prompts": {
    "web_search_query": "Instruct: Given a web search query, retrieve relevant passages that answer the query\nQuery: ",
    "sts_query": "Instruct: Retrieve semantically similar text.\nQuery: ",
    "summarization_query": "Instruct: Given a news summary, retrieve other semantically similar summaries\nQuery: ",
    "bitext_query": "Instruct: Retrieve parallel sentences.\nQuery: "
  }

In this case you still need to be aware of the different names and what they do. However it's still easier to just pick and add the correct enum value to the body and leave to TEI to add the pre-prompt than what you currently have to do so I'm in favour of adding it. I will do that quickly.

@OlivierDehaene
Copy link
Member

A first draft: #312

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 a pull request may close this issue.

2 participants