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

ADR: OpenAI + AzureOpenAI Connector #6664

Merged
merged 19 commits into from
Jun 24, 2024

Conversation

RogerBarreto
Copy link
Member

@RogerBarreto RogerBarreto commented Jun 11, 2024

OpenAI and Azure Connectors Naming and Structuring

Resolves #6529

Context and Problem Statement

It has recently been announced that OpenAI and Azure will each have their own dedicated SDKs for accessing their services. Previously, there was no official SDK for OpenAI, and our OpenAI Connector relied solely on the Azure SDK client for access.

With the introduction of the official OpenAI SDK, we now have access to more up-to-date features provided by OpenAI, making it advantageous to use this SDK instead of the Azure SDK.

Additionally, it has become clear that we need to separate the OpenAI connector into two distinct targets: one for OpenAI and another for Azure OpenAI. This separation will enhance code clarity and facilitate a better understanding of the usage of each target.

Decision Drivers

  • Avoiding breaking changes whenever possible.
  • Develop a new dedicated connector for Azure OpenAI, utilizing the underlying Azure SDK.
  • Minimize or eliminate any breaking changes for developers currently using the existing OpenAI connector.

Versioning

Although current Azure.AI.OpenAI and OpenAI SDKs have being update on its major versions, that change does not represents a SemanticKernel major breaking change, any options below consider that he new updated version of SemanticKernel.Connectors.OpenAI will be a minor version bump 1.N+1.0.

Meta Package Strategy

Currently the Microsoft.SemanticKernel package is a meta package that includes both SemanticKernel.Core and SemanticKernel.Connectors.OpenAI, with the new changes a new project will be added to the meta package SemanticKernel.Connectors.AzureOpenAI that will include the new Azure OpenAI connector.

Documentation (Upgrade Path)

A documentation guidance and samples/examples will be created to guide on how to upgrade from the current OpenAI connector to the new when needed.

OpenAI SDK limitations

The new OpenAI SDK introduce some limitations that need to be considered and pontentially can introduce breaking changes if not remediated by our internal implementation.

  • ⚠️ No support for multiple results (Choices) per request.

    Remediation: Internally make the multiple requests and combine them.
    No remediation: Breaking change removing ResultsPerPrompt from OpenAIPromptExecutionSettings.

  • ⚠️ Text Generation modality is not supported.

    Remediation: Internally provide a HttpClient to be used against gpt-3.5-turbo-instruct for text generation modality. Same way was done for TextToImage, AudioToText service modalities.
    No remediation: Breaking change removing any specific TextGeneration service implementations, this change don't impact ChatCompletion services that may still being used as ITextGenerationService implementations.

Improvements

This also represents an opportunity to improve the current OpenAI connector by introducing the Configuration pattern to allow more flexibility and control over the services and their configurations.

// Before
builder.AddAzureOpenAIChatCompletion(deploymentName, endpoint, apiKey, httpClient);
// After
builder.AddAzureOpenAIChatCompletion(new
{
    DeploymentName = modelId;
    Endpoint = endpoint;
    ApiKey = apiKey;
});
// Before
builder.AddAzureOpenAIChatCompletion(deploymentName, openAIClient, serviceId, modelId)
// After
builder.AddAzureOpenAIChatCompletion(new
{
    DeploymentName = deploymentName;
    ServiceId = serviceId;
    ModelId = modelId;
}, openAIClient);

Considered Options

  • Option 1 - Slow transition.
  • Option 2 - Independent Connectors from Start.
  • Option 3 - Update OpenAI with Azure as is.

Option 1 - Slow Transition

This is the least breaking approach where we keep the current legacy OpenAI and AzureOpenAI APIs temporarily in the connector using last Azure SDK Azure.AI.OpenAI 1.0.0-beta.17 and add new OpenAI specific APIs using the new OpenAI 2.0.0-beta.* SDK package.

This approach also implies that a new connector will be created on a second moment for Azure OpenAI services specifically fully dependent on the latest Azure.AI.OpenAI 2.0.0-beta.* SDK package.

In a later stage we will deprecate all the OpenAI and Azure legacy APIs in the SemanticKernel.Connectors.OpenAI namespace and remove Azure SDK Azure.AI.OpenAI 1.0.0-beta.17 and those APIs in a future release, making the OpenAI Connector fully dedicated for OpenAI services only depending on with the OpenAI 2.0.0-beta.* dependency.

graph TD
    A[SemanticKernel.Connectors.OpenAI] --> B[OpenAI 2.0.0-beta.*]
    A --> C[Azure.OpenAI 1.0.0-beta.17]
    D[SemanticKernel.Connectors.AzureOpenAI] --> E[Azure.AI.OpenAI 2.0.0-beta.*]
Loading

The new Options pattern we be used as an improvement as well as a measure to avoid breaking changes with the legacy APIs.

Following this change the the SemanticKernel.Connectors.OpenAI a new connector will be created SemanticKernel.Connectors.AzureOpenAI for Azure OpenAI services, using the Azure SDK Azure.AI.OpenAI 2.0.0-beta.* with all new APIs using the options approach.

Phases of the transition

  • Phase 1: Add new OpenAI SDK APIs to the current OpenAI connector and keep the Azure OpenAI APIs using the last Azure SDK.
  • Phase 2:
    • Create a new connector for Azure OpenAI services using the new Azure SDK
    • Deprecate all Azure OpenAI APIs in the OpenAI connector pointing to new AzureOpenAI connector
    • Remove Azure SDK dependency from the OpenAI connector.
    • Add AzureOpenAI connector to the Microsoft.SemanticKernel meta package.
  • Phase 3: Deprecate all legacy OpenAI APIs in the OpenAI connector pointing to new Options APIs.
  • Phase 4: Remove all legacy APIs from the OpenAI connector.

Impact

Pros:

  • Minimal breaking changes for developers using the current OpenAI connector.
  • Clear separation of concerns between OpenAI and Azure OpenAI connectors.

Cons:

  • Since SemanticKernel.Connectors.AzureOpenAI and SemanticKernel.Connectors.OpenAI share a same dependency of different versions, both packages cannot be used in the same project and a strategy will be needed when deploying both connectors:
  • Added dependency for both Azure OpenAI 1.0-beta17 and OpenAI 2.0-beta1.

Dependency Management Strategies

  1. Use only one of the connectors in the same project, some modifications will be needed to accomodate Concepts and other projects that shares OpenAI and AzureOpenAI examples.
  2. Hold AzureOpenAI connector implementation until we are ready to break (exclude) all Azure APIs in OpenAI connector.
  3. Deploy a new project with a new namespace for Azure.AI.OpenAI.Legacy 1.0.0-beta.17 and update our SemanticKernel.Connectors.OpenAI to use this new namespace to avoid version clashing on the Azure.AI.OpenAI namespace.

Option 2 - Independent Connectors from Start.

This option is focused on creating fully independent connectors for OpenAI and Azure OpenAI services since the start with all breaking changes needed to achieve that.

Impact:

  • Azure extension methods will be removed from SemanticKernel.Connectors.OpenAI namespace to avoid clashing with same names introduced in the new SemanticKernel.Connectors.AzureOpenAI.

  • All azure specific services like AzureOpenAIChatCompletion, AzureOpenAITextGeneration services will be obsoleted and throw not supported exceptions when used from SemanticKernel.Connectors.OpenAI connector.

Impact

Pros:

  • Clear separation of concerns between OpenAI and Azure OpenAI connectors.
  • Small breaking changes for developers focused on OpenAI specific APIs.
  • Faster transition to the new OpenAI SDK and Azure OpenAI SDK.

Cons:

  • Large breaking changes for developers using the current OpenAI connector for Azure.
  • ⚠️ May have the same problem with the dependency management as in Option 1 when using both connectors in the same project may conflict due to the shared dependency of different OpenAI SDK versions.

Option 3 - Update OpenAI with Azure as is.

This option is fully focused in the least impact possible, combining both Azure and OpenAI SDK dependencies in one single connector following the same approach as the current connector.

Changes:

  1. Update all current OpenAI specific services and client to use new OpenAI SDK
  2. Update Azure specific services and client to use the latest Azure OpenAI SDK.
  3. Optionally add Options pattern new APIs to the connector services and deprecate old ones.

Impact

Pros:

  • Minimal breaking changes for developers using the current OpenAI connector.
  • The breaking changes will be limited on how we tackle the points mentioned in the OpenAI SDK limitations above.

Cons:

  • We will be limited on the OpenAI SDK version that is used by the latest Azure.AI.OpenAI package, which may not be the latest version available.
  • Developers using direct OpenAI services don't expected having any Azure related dependencies.

Decision Outcome

Chosen option:

TBD

OpenAI SDK limitations:

  • Multiple results: Do not remediate.
  • Text Generation modality is not supported: Do not remediate.

@stephentoub
Copy link
Member

stephentoub commented Jun 11, 2024

As the whole underpinnings here are changing, I suggest we do accept breaking changes here, and that we:

  • Separate the OpenAI connector from the Azure OpenAI connector; two separate set of types, extension methods, packages.
  • Update the public APIs that take OpenAIClient to instead take the appropriate library-specific type.
  • Refactor the internal implementation to be better aligned with the individual client types.
  • Take this opportunity when stuff is breaking to fix up other things that are slightly breaking, e.g. making the prompt execution setting properties that are currently non-nullable but should be nullable instead be nullable.
  • Align any naming with any revised naming in the underlying libraries.
  • Etc.

We should also put in place a changelog that includes notable changes from release to release, including notes on breaking changes and what specifically someone should do to address the change.

I obviously don't take breaking changes lightly, but this is one of those times when it's warranted.

@Josephrp
Copy link

it could be an opportunity to handle more functionality in these clients , like cost estimation logprobs and feedback , good luck on this one, it's not a trival thing to update and it also affect environment variables , so it's a big deal actually .

@joslat
Copy link
Contributor

joslat commented Jun 17, 2024

I tend on the opinion of @stephentoub here. more things will be added to the API And also what @Josephrp states too. Two APIs that evolve separately (even they will converge eventually) and have their own SDKs. Option 2 then is best IMHO.
Option 3 does not compute and Option 1 is not as clean and architecturally nice as Option 2.

@RogerBarreto RogerBarreto self-assigned this Jun 19, 2024
@dluc
Copy link
Collaborator

dluc commented Jun 21, 2024

If there's room for it, it would be nice to rename top_p/TopP to "nucleus sampling"

@RogerBarreto
Copy link
Member Author

@dluc As top_p is something provider specific I would keep the name that the provider uses, and only use a Nucleus Sampling name if the API does that. Our PromptExecutionSettings abstractions don't have top_p parameter.

On the subject of TopP being called as NucleusSampling I'm afraid this is not what we are seeing in the APIs for AI currently and I suggest we keep our names in line with is most common.

Different APIs refering as top_p.

@RogerBarreto RogerBarreto added this pull request to the merge queue Jun 24, 2024
Merged via the queue into microsoft:main with commit 5d3cbed Jun 24, 2024
12 checks passed
@RogerBarreto RogerBarreto deleted the adrs/openai-connectors branch June 24, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

.Net: ADR covering the migration to OpenAI SDK
7 participants