Allow user to provide their own llm client

[samgregson]

I'm not a fan of the approach whereby the 'Engine' is created from scratch. It would be better if the user passed in an llm client to an adaptor function.

E.g.

client = OpenAI()
engine = Engine.fromOpenAI(client)

Why?

1. users could use libraries like instructor and langsmith, as these wrap or patch llm clients.
2. you / contributors wouldn't have to support a many providers as there would be more overlap.
3. you / contributors wouldn't have to handle errors like missing api keys

Additionally, why not portobello a way for users to add their own custom engine as a Callable -> str

def myLM(s: str):
    return f"hi, you wrote: {s}"
engine = Engine.fromCustom(myLM)

I'd be happy to give this a go but wanted to check for whether people thought this was a good idea first.

[mertyg]

I think this looks good. @vinid had an initial WIP PR here, which had the similar goal of reducing the implementation work to bring-your-own-engine type thing.

I really like what you're suggesting here too! I'm fine with either approach, whichever you two decide is useful.

[samgregson]

maybe it could be something as simple as this, don't know. But this would introduce breaking changes!? (obviously just showing for OpenAI here).

Then you also wouldn't need AzureOpenAI.

is_multimodal also seems redundant, surely errors would be produced by the client, let the provider handle these errors, they are not your concern.
I think this would just be cleaner, less maintenance, and allow more flexibility.

In fact, you have basically implemented this already with class ChatExternalClient(ChatOpenAI):. So this would be 3 classes reduced to 1. Don't tell me that a coder doesn't know how to write client = OpenAI().

[samgregson]

I personally think this is a better solution but wonder what people think of introducing breaking changes?

[vinid]

@mertyg what do you think?

[mertyg]

i think this looks reasonable, but I guess we can do this without breaking changes. i.e., if a string is provided, use the old way, if a client is provided, use the new way.

does that make sense?

[samgregson]

well even if you have a client you still need a string as the client is just the provider, e.g. an OpenAI client can have different models. I think if you don't want breaking changes then you should probably just stick to what you have, OR write new methods and deprecate the old ones to give people time to transition. I think keeping both just gets messy. But that's of course just my opinion.

[samgregson]

...but I see that most pull requests are about adding support for new providers even when they have the same models (openai family, anthropic family, etc.). Moving in the direction of the user providing client would cut the amount calls for supporting providers and pull requests as the family implementation remains the same even if the provider changes.