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

Adding Azure Search parameters to the payload in the AOAI Chat #1178

Open
1 task done
joergrau opened this issue May 26, 2024 · 9 comments
Open
1 task done

Adding Azure Search parameters to the payload in the AOAI Chat #1178

joergrau opened this issue May 26, 2024 · 9 comments
Assignees
Labels
Bug Something isn't working

Comments

@joergrau
Copy link

Describe the issue

Small product enhancement:
The Codeunit "AOAI Chat Completion Params" uses the Codeunit "AOAI Chat Compl Params Impl", which adds parameters to the payload:
procedure AddChatCompletionsParametersToPayload(var Payload: JsonObject)
begin
if GetMaxTokens() > 0 then
Payload.Add('max_tokens', GetMaxTokens());
Payload.Add('temperature', GetTemperature());
Payload.Add('presence_penalty', GetPresencePenalty());
Payload.Add('frequency_penalty', GetFrequencyPenalty());
end;

There is currently no option to specify Azure Search parameters. The parameters I would like to add are:
"azureSearchEndpoint"
"azureSearchKey"
"azureSearchIndexName"

Expected behavior

Adding additional procedures to set and get the new Azure Seach parameters. Adding the parameters to the payload if they were set.

Steps to reproduce

Currently no procedures available to set the Azure Search parameters.

Additional context

As discussed on the yammer group: https://www.yammer.com/dynamicsnavdev/#/threads/inGroup?type=in_group&feedId=132956700672&view=all

I will provide a fix for a bug

  • I will provide a fix for a bug
@joergrau joergrau added the Bug Something isn't working label May 26, 2024
joergrau added a commit to joergrau/BCApps that referenced this issue May 26, 2024
@darjoo
Copy link
Contributor

darjoo commented May 27, 2024

Hi @joergrau, this is something we'd accept :)
I need a bit more clarification about your implementation. Is this purely just adding the parameters to enable the it to perform Azure AI Search or would there be additions to the API/Objects that make it easier to work with the information returned as well?

-Darrick

@joergrau
Copy link
Author

Hi @darjoo,
It is for adding more procedures for set/get the three parameters and adding them to the payload like this:

procedure AddChatCompletionsParametersToPayload(var Payload: JsonObject)
    begin
        if GetMaxTokens() > 0 then
            Payload.Add('max_tokens', GetMaxTokens());

        Payload.Add('temperature', GetTemperature());
        Payload.Add('presence_penalty', GetPresencePenalty());
        Payload.Add('frequency_penalty', GetFrequencyPenalty());

        if IsJsonMode() then
            Payload.Add('response_format', GetJsonResponseFormat());

        if GetAzureSearchEndpoint() <> '' then
            Payload.Add('azureSearchEndpoint', GetAzureSearchEndpoint());
        if GetAzureSearchKey() <> '' then
            Payload.Add('azureSearchKey', GetAzureSearchKey());
        if GetAzureSearchIndexName() <> '' then
            Payload.Add('azureSearchIndexName', GetAzureSearchIndexName());
    end;

So, following the same way as for the other parameters, e.g. temperature

@darjoo
Copy link
Contributor

darjoo commented Jun 3, 2024

@joergrau
Just adding those parameters do not seem correct though. I have not tested and only reviewed the documentation and API specs.
It seems like it should be added through the DataSource key, with a specific type etc.

https://learn.microsoft.com/en-us/azure/ai-services/openai/references/on-your-data?tabs=python#data-source
https://github.com/Azure/azure-rest-api-specs/blob/main/specification/cognitiveservices/data-plane/AzureOpenAI/inference/stable/2024-02-01/inference.json

And even once that works, it would be better for all if we can work together to design how the output can be worked with. For example with Operation Response/Function Response. Otherwise, everyone who uses this will have to develop their own parsing mechanism which makes little sense when it should be part of the AI module.

@joergrau
Copy link
Author

joergrau commented Jun 6, 2024

@darjoo I already did the coding as it is not much work. However, I struggle to get this tested as I can't do any testing around AI in a Docker container. I tried to follow the process explained here LOCAL_DEV_ENV.md get an error message:
image

when running the NewDevEnv.ps1 script.

What is the best way to deploy this updated System App and to try it out?

@JesperSchulz
Copy link
Contributor

@mazhelez / @aholstrup1, could you look into the script issue?

@mazhelez
Copy link
Contributor

@joergrau I see codeunit "Library - Password" was added recently. Are you maybe not running against the latest code or latest artifact?

@joergrau
Copy link
Author

@mazhelez I synced my fork and updated my branch, but I got the same error.

@JesperSchulz
Copy link
Contributor

@darjoo / @joergrau, where are we on this one?

@joergrau
Copy link
Author

@JesperSchulz I'm waiting for your assistance. Alternatively, please advise on how I could deploy a new system app to test AI integration. The coding is done, I "just" need to test it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants