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

.Net: Introduce IAIServiceSelector interface to allow AI service and request settings to be selected during semantic function execution #3227

Conversation

markwallace-microsoft
Copy link
Member

@markwallace-microsoft markwallace-microsoft commented Oct 18, 2023

Motivation and Context

Fix following issues for Semantic functions:

  1. Allow semantic functions to dynamically retrieve the AI service and request settings to be used when executing the function

Previously semantic functions execution has the following limitations:

  1. The AI service was set on the semantic function when it was created. This meant that an instance of a semantic function could not be used with multiple different Kernel instances.
  2. The AI request settings were set on the semantic function when it was created. This meant that different request settings could not be selected when the function was executed.

Description

This PR add's a new abstraction called IAIServiceSelector. An instance of this is added to a semantic function when it is created. The default implementation works as follows:

  1. If the semantic function only has a single associated model request setting instance with no service id then the default AI service and the model request settings are used. This is consistent with the previous behaviour.
  2. If the semantic function only has a single associated model request setting instance with a service id then the named AI service and the model request settings are used. If the named AI service is not available an exception will be thrown. This is consistent with the previous behaviour.
  3. If the semantic function only has multiple associated model request setting instances then:
    1. The model request setting instances are considered in order
    2. The first model request setting instance with no service id is considered the default
    3. For each model request setting instance that has a service id, if the service exists then it will be used with the associated model request settings
    4. If no matching service can be found and default request settings are provided the default service will be used
    5. If no matching service can be found and no default request settings are provided an exception will be thrown

Contribution Checklist

@shawncal shawncal added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core labels Oct 18, 2023
@github-actions github-actions bot changed the title Add factories for AI services and request settings .Net: Add factories for AI services and request settings Oct 18, 2023
@markwallace-microsoft markwallace-microsoft force-pushed the users/markwallace/ai_factories branch from 719c43e to 93032e9 Compare October 18, 2023 20:01
@markwallace-microsoft markwallace-microsoft changed the title .Net: Add factories for AI services and request settings .Net: Introduce IAIServiceSelector interface to allow AI service and request settings to be selected during semantic function execution Oct 20, 2023
@markwallace-microsoft markwallace-microsoft marked this pull request as ready for review October 20, 2023 13:35
@markwallace-microsoft markwallace-microsoft requested a review from a team as a code owner October 20, 2023 13:35
@markwallace-microsoft markwallace-microsoft self-assigned this Oct 23, 2023
@markwallace-microsoft markwallace-microsoft linked an issue Oct 23, 2023 that may be closed by this pull request
Copy link
Member

@RogerBarreto RogerBarreto left a comment

Choose a reason for hiding this comment

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

Nit comments.
Suggestion on changing SelectAIService to aways throw if no service is found. Removing if there's any Custom logic that handling ITextCompletion being nullj in the SemanticFunction InvokeAsync impl.

Maybe add just Verify.IsNull()

Overall LGTM

@markwallace-microsoft markwallace-microsoft added this pull request to the merge queue Oct 23, 2023
Merged via the queue into microsoft:main with commit 3601022 Oct 23, 2023
18 checks passed
@markwallace-microsoft markwallace-microsoft deleted the users/markwallace/ai_factories branch October 23, 2023 22:19
SOE-YoungS pushed a commit to SOE-YoungS/semantic-kernel that referenced this pull request Nov 1, 2023
…request settings to be selected during semantic function execution (microsoft#3227)

### Motivation and Context

Fix following issues for Semantic functions:

1. Allow semantic functions to dynamically retrieve the AI service and
request settings to be used when executing the function

Previously semantic functions execution has the following limitations:

1. The AI service was set on the semantic function when it was created.
This meant that an instance of a semantic function could not be used
with multiple different Kernel instances.
1. The AI request settings were set on the semantic function when it was
created. This meant that different request settings could not be
selected when the function was executed.

### Description

This PR add's a new abstraction called `IAIServiceSelector`. An instance
of this is added to a semantic function when it is created. The default
implementation works as follows:

1. If the semantic function only has a single associated model request
setting instance with no service id then the default AI service and the
model request settings are used. This is consistent with the previous
behaviour.
1. If the semantic function only has a single associated model request
setting instance with a service id then the named AI service and the
model request settings are used. If the named AI service is not
available an exception will be thrown. This is consistent with the
previous behaviour.
1. If the semantic function only has multiple associated model request
setting instances then:
    1. The model request setting instances are considered in order
1. The first model request setting instance with no service id is
considered the default
1. For each model request setting instance that has a service id, if the
service exists then it will be used with the associated model request
settings
1. If no matching service can be found and default request settings are
provided the default service will be used
1. If no matching service can be found and no default request settings
are provided an exception will be thrown

### Contribution Checklist

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

.Net: Use Multiple Models in a Single Kernel
4 participants