microsoft
Abstract current OpenAI and AzureOpenAI implementations from the Core Kernel
Motivation and Context
This is an effort to abstract current OpenAI and AzureOpenAI implementations from the Core Kernel code base.
-
Why is this change required? This change increases the abstractions on the Core and allow it to be handling any Backend implementations in the future
-
What problem does it solve? Currently the Kernel is very coupled with those two implementations and any effort to do something differently demands more code and complexity
-
What scenario does it contribute to? This contributes to reducing the places in the code that need to be changed to support new or any change in the current configurations
Description
The changes made includes abstracting how backend configurations are added to the KernelConfig class making use of two new flag interfaces ICompletionBackendConfig and IEmbeddingsBackendConfig.
Another change that was necessary was inverting the control on the ITextCompletionClient instantiation in Kernel CreateSemanticFunction to the KernelConfig as a delegate when a func.SetAIBackend is defined.
With those changes it was possible to move out of both Kernel and KernelConfig any specific implementations from AzureOpenAI and OpenAI and adding them as Extension methods in KernelConfigExtensions.cs.
This makes the code simpler and prepares a clear path if we need to move those implementations to a dedicated backend package and add many more implementations.
Added unit tests to validate the current behavior as expected without impacts.
Contribution Checklist
- [x] The code builds clean without any errors or warnings
- [x] The PR follows SK Contribution Guidelines (https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [x] The code follows the .NET coding conventions (https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions) verified with
dotnet format - [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone :smile:
@RogerBarreto some conflicts to address. There's also another PR touching the HTTP layer, so could happen again, in one or the other, depending on the order these will be merged
100% agree with the premise and in fact I recently started drafting a doc to think through this holistically. I was trying to integrate a separate model and noticed the excessive effort required.
Even if we merged this PR the problem would persist, e.g. the kernel is still rooted on text and completions (semantic memory is the only exception so far; and connectors are tangentially part of this conversation, e.g. OAI clients == connectors).
We need to push further and reason about more generic and divers AI skills, and how the kernel allows to integrate them in the same programming model.
My suggestion would be to park this PR, and join asap the conversation about the goals, and at the same time extend the list of models, making these changes part of the same story:
- decouple from completion
- integrate OAI ChatGPT, Edit, Insert, DallE, Stable Diffusion, Azure Speech
- allow to use any backend, and for each backend one must provide specs and clients
Adding more models is the only way to test if the new code can scale. Otherwise, I would say that we're just moving the code around, without verifying the benefits. (This would also be something like the 8th/9th time we refactor OpenAI clients integration, I think this is a good opportunity to extend the AI surface as planned).
@dluc, I agree, will follow your suggestions.
Let's leave this PR parked for now, since there are some core changes that can be added as part of it.
The current existing conflics were resolved.