eval-dev-quality icon indicating copy to clipboard operation
eval-dev-quality copied to clipboard

Generic OpenAI API provider

Open bauersimon opened this issue 1 year ago • 4 comments

Everyone is using the OpenAI API. Hence, we want to have a generic OpenAI API provider that enables one to use custom endpoints even if we don't have a native provider for that endpoint.

We use go-openai which allows to adapt to an endpoint via:

  • specifying a custom endpoint URL (which we already do internally for openrouter.ai)
  • specifying an access token (which is used as HTTP header for authentification)
  • specifying the model one wants to query...

To use our new generic OpenAI API provider, we need to make these options available via the CLI, i.e.:

eval-dev-quality evaluate \
  --urls custom-provider-a:https://a.example.com/api,custom-provider-b:https://b.example.com/api
  --tokens custom-provider-a:abcdefg,custom-provider-b:scdefgh
  --model custom-provider-a/modelA
  --model custom-provider-b/modelB

Internally, this should be implemented as follows:

  • [x] add generic provider/openai-api API provider with custom url, token + []Model
  • [x] before evaluation
    • [x] go through CLI argument urls and look at the ones with the custom- prefix: provider.Register(openaiapi.NewProvider(<url>, <name>))
    • [x] go through CLI arguments model and add the models to their matching custom provider's []Model
  • [x] token injection should work automatically since the generic providers are registered properly
  • [ ] test this with i.e. an Ollama model because we have Ollama (soon #27)
  • [ ] refactor the existing openrouter.ai and Ollama providers so they internally use this generic provider as base

  • Q: But with this approach we are skipping the actually model querying because we are always blindly registering the models that are specified in the CLI :angry:.
  • A: Yes but sadly we cannot be sure the endpoint supports model querying (i.e. Ollama supports only the OpenAI API prompt querying but not the model listing API). So for now just trust that the models exist. Maybe we can add the check in the future but then need to add a way to disable again for cases like Ollama.

bauersimon avatar May 14 '24 09:05 bauersimon

CC @Munsio @zimmski

bauersimon avatar May 14 '24 09:05 bauersimon

  • Would throw an error when a name clashes, already existing provider should take precedence (is that even possible? we prefix the "custom" part so there shouldn't be any clashes anyway)
  • That approach would also allow for something like: --urls ollama:https://not-localhost which I do like, so you could use the ollama provider and say it should use something with more power instead of local execution
  • skip querying all available models we could register the "models" based on --model custom-provider-a/modelX like we do with the provider. With that a call to custom-provider.models() would return modelX or even multiple if you want to run multiple different models on the same URL (If that is even possible)

Sound good for me.

Munsio avatar May 14 '24 09:05 Munsio

I was thinking of not enforcing a special prefix for these new providers... would make it a bit prettier and nicer to specify them. But indeed the clashes with existing providers are tricky. And with the prefix we never clash plus we could in the future easily change the Ollama url easily.

Thanks for the suggestion of registering the models from the CLI and then having the model listing run as usual. Makes everything simpler (even though the listing and validating models is pointless then because the listed ones are the ones specified).

bauersimon avatar May 14 '24 09:05 bauersimon

(even though the listing and validating models is pointless then because the listed ones are the ones specified).

Only for the custom provider which I would say is by default as /models is not supported by all LLM providers. But it would still make everything simpler. And all other providers do validate against their set of available models so no problem there I guess?

Munsio avatar May 14 '24 10:05 Munsio