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: Memory Plugin Package - Initial Copy #2278

Closed

Conversation

shawncal
Copy link
Member

@shawncal shawncal commented Aug 2, 2023

Merging into FEATURE BRANCH

Memory is already a "skill" (TextMemorySkill) in Semantic Kernel. It also has many classes, interfaces, and extensions baked into, and exposed by, various layers of the SDK. This can be problematic, as the interfaces we specify are just one perspective of what "Memory" and a memory plugin could be. Other implementations may treat memory storage and retrieval very differently, and the Kernel core layer (orchestration, execution) should be none the wiser.

This PR seeks to decouple this implementation of a memory skill (plugin) from the Kernel Core by introducing it as a new package. Note: this package, being one of our recommended set, will be included in the "Microsoft.SemanticKernel" metapackage, so developers and AIs will continue to have access to it as they currently do.

Description

This is the first of a multi-stage PR. First:

  • Creating a new Plugins.Memory package
  • COPY the TextMemorySkill from Skills.Core
  • COPY the other necessary memory components from Kernel.Abstractions and Kernel.Core, into this plugin package where they'll be used.
  • Created a new Plugins.Memory.UnitTests project, and COPIED all memory-related UTs into this package

Embeddings:

  • Created a new AI.Embeddings utilities package which contains only the Embedding structs and vector math (nothing using SK)
  • The Embedding struct and vector math was MOVED from .Core into this package, and project references added.

Next steps:

  • Update the Connectors.Memory.* package to use this Plugins.Memory instead of the SK Memory classes
  • Remove the Memory classes from SK.Abstractions and Core (those used only by the plugin)
  • Update samples, apps

@shawncal shawncal added .NET Issue or Pull requests regarding .NET code feature branch PR targeting a feature branch memory connector labels Aug 2, 2023
@shawncal shawncal requested a review from awharrison-28 August 2, 2023 00:28
@shawncal shawncal requested a review from a team as a code owner August 2, 2023 00:28
@stephentoub
Copy link
Member

Created a new AI.Embeddings utilities package which contains only the Embedding structs and vector math (nothing using SK)

FYI, I'm working on a PR where I'm proposing to delete all of the Embedding types.

We're also working on dotnet/runtime#89639, and we should be able to then delete the math APIs from SK as well.

@shawncal
Copy link
Member Author

shawncal commented Aug 2, 2023

Created a new AI.Embeddings utilities package which contains only the Embedding structs and vector math (nothing using SK)

FYI, I'm working on a PR where I'm proposing to delete all of the Embedding types.

We're also working on dotnet/runtime#89639, and we should be able to then delete the math APIs from SK as well.

Awesome. I think I may wind back parts of this change for a simpler, more staged approach. I'll undo the packaging of the embeddings stuff.

@shawncal shawncal marked this pull request as draft August 2, 2023 16:12
@shawncal shawncal closed this Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature branch PR targeting a feature branch .NET Issue or Pull requests regarding .NET code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants