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: Implement or remove unused planner options #4728

Closed
gitri-ms opened this issue Jan 24, 2024 · 0 comments · Fixed by #4793
Closed

.Net: Implement or remove unused planner options #4728

gitri-ms opened this issue Jan 24, 2024 · 0 comments · Fixed by #4793
Assignees
Labels
.NET Issue or Pull requests regarding .NET code planner Anything related to planner or plans sk team issue A tag to denote issues that where created by the Semantic Kernel team (i.e., not the community)

Comments

@gitri-ms
Copy link
Contributor

gitri-ms commented Jan 24, 2024

Some options, such as GetFunctionCallback, are not currently used by either planner. These should either be implemented or removed.

@gitri-ms gitri-ms added .NET Issue or Pull requests regarding .NET code planner Anything related to planner or plans sk team issue A tag to denote issues that where created by the Semantic Kernel team (i.e., not the community) labels Jan 24, 2024
@gitri-ms gitri-ms self-assigned this Jan 24, 2024
@github-actions github-actions bot changed the title Implement or remove unused planner options .Net: Implement or remove unused planner options Jan 24, 2024
@gitri-ms gitri-ms linked a pull request Jan 30, 2024 that will close this issue
4 tasks
@gitri-ms gitri-ms linked a pull request Jan 30, 2024 that will close this issue
4 tasks
github-merge-queue bot pushed a commit that referenced this issue Feb 6, 2024
### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->
Changes to improve consistency across the .NET planners. Addresses the
following:
- #4727 
- #4728 
- #4729 
- #4731 
- #4775 

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
- Clean up planner options. Shared options live in parent class
(`PlannerOptions`). Other options reside in the planner-specific options
class (`HandlebarsPlannerOptions` or
`FunctionCallingStepwisePlannerOptions`).
- Change default name delimiter to dash (was underscore for function
calling stepwise planner). Update all tests accordingly. (Will explore
making this configurable in a separate issue/PR)
- Change how we get the list of available functions for Handlebars
planner, to reduce code duplication and get overridable options for
free.
- Make `MaxTokens` setting optional for function calling stepwise
planner (not yet supported for Handlebars planner)

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [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 😄

---------

Co-authored-by: Chris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.NET Issue or Pull requests regarding .NET code planner Anything related to planner or plans sk team issue A tag to denote issues that where created by the Semantic Kernel team (i.e., not the community)
Projects
Archived in project
3 participants