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

Add Azure OpenAI Chat and Embedding Options #286

Closed

Conversation

tzolov
Copy link
Contributor

@tzolov tzolov commented Jan 30, 2024

  • Add AzureOpenAiChatOptions
    • Add default options field to AzureOpenAiChatClient.
    • Impl runtime (e.g. prompt) and default options on call.
    • Add options field to the AzureOpenAiChatProperties.
  • Add AzureOpenAiEmbeddingOptions
    • Add default options field to AzureOpenAiEmbeddingClient.
    • Implement runtime and default option merging on embedding request.
    • Add options field to AzureOpenAiEmbeddingProperties.
  • Add Unit and ITs.
  • Split the azure-openai.adoc into ./clients/azure-openai-chat.adoc and ./embeddings/azure-openai-embeddings.adoc.
    • Provide detailed explanation how to use the chat and embedding clients manually or via the auto-configuration.

@tzolov tzolov force-pushed the add-azure-chat-embedding-options branch from 4b88b5c to c2dbc31 Compare January 30, 2024 20:45
@tzolov tzolov added this to the 0.8.0 milestone Jan 31, 2024
@tzolov tzolov force-pushed the add-azure-chat-embedding-options branch 2 times, most recently from fe7ebf8 to 8c99ea3 Compare February 2, 2024 14:55
 - Add AzureOpenAiChatOptions
   - Add default options field to AzureOpenAiChatClient.
   - Impl runtime (e.g. prompt) and default options on call.
   - Add options field to the AzureOpenAiChatProperties.
 - Add AzureOpenAiEmbeddingOptions
   - Add default options field to AzureOpenAiEmbeddingClient.
   - Impmlement runtime and default option merging on embedding request.
   - Add options field to AzureOpenAiEmbeddingProperties.
 - Add Unit and ITs.
 - Split the azure-openai.adoc into ./clients/azure-openai-chat.adoc and ./embeddings/azure-openai-embeddings.adoc.
   - Provide detailed explanation how to use the chat and embedding clients manually or via the auto-configuration.
@tzolov tzolov force-pushed the add-azure-chat-embedding-options branch from 8c99ea3 to 2470f54 Compare February 3, 2024 14:10
@markpollack
Copy link
Member

review this in the context of #10

Copy link
Member

@markpollack markpollack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will merge, but let's circle back on the comments in another issue/iteration.
Tx!

@@ -1,78 +1,9 @@
# 1. Azure OpenAI
# Azure OpenAI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not remove this readme... docs should be centralized.

mergedAzureOptions.setTemperature(azureOptions.getTemperature());
if (mergedAzureOptions.getTemperature() == null && springAiOptions.getTemperature() != null) {
mergedAzureOptions.setTemperature(springAiOptions.getTemperature().doubleValue());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have if (mergedAzureOptions.getTemperature() == null && springAiOptions.getTemperature() != null) {

instead of if (mergedAzureOptions.getTemperature() == null as in the other cases?

Also we don't need to call doubleValue() in mergedAzureOptions.setTemperature(springAiOptions.getTemperature().doubleValue()); as it accepts the wrapper number type directly.

@markpollack
Copy link
Member

merged as dfbea8c

@markpollack markpollack closed this Feb 5, 2024
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 this pull request may close these issues.

2 participants