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

TextMemorySkill: Recall Can Return Many Memories, added Keyed Lookup Functions Retrieve() and Remove() #35

Merged
merged 35 commits into from
Mar 20, 2023

Conversation

awharrison-28
Copy link
Contributor

@awharrison-28 awharrison-28 commented Mar 8, 2023

Motivation and Context

Currently, TextMemorySkill Recall only returns the most relevant memory despite being reliant on methods that support returning N relevant memories.

Additionally, users should be able to Remove memories.

Description

This PR introduces new terminology to TextMemorySkill: Remove and Retrieve.

Remove and 'Retrieve' allows the TextMemorySkill get and delete memories from storage by unique key (since memories are stored as key-value pairs).

Users may Retrieve/Remove a single memory using the unique key associated with the memory.

Redefined function:

  • Recall -> look up a number of memories relevant to a given idea string. The number of results depends on a LimitParam and RelevanceParam provided by the SKContext. The results text records are concatenated to a single Json string.

New functions:

  • RetrieveAsync -> look up a specific memory associated with a unique key.
  • RemoveAsync -> delete a specific memory associated with a unique key.

Additional Changes:

  • Modified KernelSyntaxExample Example15_MemorySkill to use the new functions and features.
  • Added RemoveAsync to ISemanticTextMemory and its implementors SemanticTextMemory and NullMemory

Note: If the result of 'Recall' will be used for a completion skill, this could result in unexpected behavior if the text for N memories exceeds the model's token limit. Addressing input chunking to a completion model is not in scope for this PR.

@awharrison-28 awharrison-28 added the PR: ready for review All feedback addressed, ready for reviews label Mar 8, 2023
@awharrison-28 awharrison-28 changed the title TextMemorySkill: Remember<->ForgetMemory, Recall<->ForgetIdea TextMemorySkill: Recall<->Forget, RecallIdea<->ForgetIdea Mar 8, 2023
@alexchaomander
Copy link
Contributor

@awharrison-28 if there are any breaking changes, can we also get them reflected in the memory notebook?

https://github.com/microsoft/semantic-kernel/blob/main/samples/notebooks/dotnet/6-memory-and-embeddings.ipynb

@awharrison-28
Copy link
Contributor Author

@alexchaomander great call out. Will update shortly.

@awharrison-28 awharrison-28 added PR: feedback to address Waiting for PR owner to address comments/questions and removed PR: ready for review All feedback addressed, ready for reviews labels Mar 8, 2023
@awharrison-28 awharrison-28 added PR: ready for review All feedback addressed, ready for reviews and removed PR: feedback to address Waiting for PR owner to address comments/questions labels Mar 8, 2023
@awharrison-28
Copy link
Contributor Author

@alexchaomander I updated the notebook to address breaking changes as is. We should update notebook 6 to reflect new TextMemorySkill features, but I believe that should be a separate PR.

@awharrison-28 awharrison-28 changed the title TextMemorySkill: Recall<->Forget, RecallIdea<->ForgetIdea TextMemorySkill: RecallMemory<->ForgetMemory, RecallIdea<->ForgetIdea Mar 8, 2023
@awharrison-28 awharrison-28 requested a review from dluc March 16, 2023 18:20
Copy link
Contributor

@glahaye glahaye left a comment

Choose a reason for hiding this comment

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

There's a couple of variables that are assigned with the wrong data...

@dluc dluc removed the request for review from shawncal March 20, 2023 16:52
@adrianwyatt adrianwyatt enabled auto-merge (squash) March 20, 2023 18:14
@adrianwyatt adrianwyatt merged commit 283a02a into microsoft:main Mar 20, 2023
@awharrison-28 awharrison-28 deleted the textmemoryskill_improvements branch March 21, 2023 17:50
dehoward pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023
…Functions Retrieve() and Remove() (microsoft#35)

### Motivation and Context
Currently, TextMemorySkill `Recall` only returns the most relevant
memory despite being reliant on methods that support returning N
relevant memories.

Additionally, users should be able to `Remove` memories. 

### Description
This PR introduces new terminology to TextMemorySkill: `Remove` and
`Retrieve`.

`Remove` and 'Retrieve' allows the TextMemorySkill get and delete
memories from storage by unique key (since memories are stored as
key-value pairs).

Users may Retrieve/Remove a single memory using the unique key
associated with the memory.

Redefined function:

- `Recall` -> look up a number of memories relevant to a given idea
string. The number of results depends on a `LimitParam` and
`RelevanceParam` provided by the SKContext. The results text records are
concatenated to a single Json string.

New functions:

- `RetrieveAsync` -> look up a specific memory associated with a unique
key.
- `RemoveAsync` -> delete a specific memory associated with a unique
key.

Additional Changes:

- Modified KernelSyntaxExample `Example15_MemorySkill` to use the new
functions and features.
- Added `RemoveAsync` to `ISemanticTextMemory` and its implementors
`SemanticTextMemory` and `NullMemory`

**Note:** If the result of 'Recall' will be used for a completion skill,
this could result in unexpected behavior if the text for N memories
exceeds the model's token limit. Addressing input chunking to a
completion model is not in scope for this PR.
golden-aries pushed a commit to golden-aries/semantic-kernel that referenced this pull request Oct 10, 2023
### Motivation and Context

<!-- Thank you for your contribution to the copilot-chat 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.
-->
Add a persona tab to show/enable the following:
1. Meta prompt.
2. Meta prompt editing,
3. Memory (long term & short term) content.
4. Memory bias slider.

### Description
1. Webapi support for editing the meta prompt.
2. Webapi support for retrieving memory content (ChatMemoryController).
3. Webapi support for setting memory bias.
4. Webapp support for showing and enabling the features.
5. Update the initial bot greeting message.

![image](https://github.com/microsoft/chat-copilot/assets/12570346/8ac7f817-bcab-4b71-98c7-03f3afc3b8f9)

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

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

- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready for review All feedback addressed, ready for reviews PR: ready to merge PR has been approved by all reviewers, and is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants