AutoGPT icon indicating copy to clipboard operation
AutoGPT copied to clipboard

Add memory backend using sqlite and zarr

Open Tymec opened this issue 1 year ago • 13 comments

Background

The purpose of this change is to provide a more efficient and flexible memory backend for storing memories. The proposed memory backend uses SQLite and Zarr to store data and embeddings locally in a compressed and chunked format. The implementation is based on the work of wawawario2/long_term_memory. The implementation can be easily adapted to support the local embeddings model implemented in #1320. The change is related to the issue #430, where the idea of using SQLite a memory backend was suggested.

Changes

  • Implemented a new memory backend that uses SQLite and Zarr for storing data and embeddings as an alternative to Redis or Pinecone
  • To use, either set the config option memory_embeder or the environment variable MEMORY_EMBEDER to sqlite.
  • Updated requirements.txt with zarr and sklearn dependencies
  • Added /memory/ to .gitignore to avoid committing the database and embeddings files

Documentation

  • The code changes are documented using docstrings and comments

Test Plan

  • Created a unit test module to test if everything works as intended.
  • Further testing might be required for out of sync issues between the database and embeddings

PR Quality Checklist

  • [x] My pull request is atomic and focuses on a single change.
  • [x] I have thoroughly tested my changes with multiple different prompts.
  • [x] I have considered potential risks and mitigations for my changes.
  • [x] I have documented my changes clearly and comprehensively.
  • [x] I have not snuck in any "extra" small tweaks changes

Tymec avatar Apr 15 '23 01:04 Tymec

Also, CI is red

nponeccop avatar Apr 16 '23 15:04 nponeccop

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

github-actions[bot] avatar Apr 17 '23 16:04 github-actions[bot]

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

github-actions[bot] avatar Apr 17 '23 17:04 github-actions[bot]

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

github-actions[bot] avatar Apr 18 '23 14:04 github-actions[bot]

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

github-actions[bot] avatar Apr 18 '23 14:04 github-actions[bot]

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

github-actions[bot] avatar Apr 18 '23 22:04 github-actions[bot]

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

github-actions[bot] avatar Apr 19 '23 17:04 github-actions[bot]

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

github-actions[bot] avatar Apr 19 '23 19:04 github-actions[bot]

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

github-actions[bot] avatar Apr 23 '23 16:04 github-actions[bot]

This is a mass message from the AutoGPT core team. Our apologies for the ongoing delay in processing PRs. This is because we are re-architecting the AutoGPT core!

For more details (and for infor on joining our Discord), please refer to: https://github.com/Significant-Gravitas/Auto-GPT/wiki/Architecting

p-i- avatar May 05 '23 00:05 p-i-

Hey thanks for the Pull Request! I am gonna review this more thoroughly momentarily. My gut reaction is this looks like pretty good code, and you should talk to @p-i- Pi in discord for a contributor badge. However, we are going to be moving all of our memory systems to plugins except for one or two of them and that is actively being refactored.

I'm gonna check with other folks, but it might make sense to put the sqllite piece of this on hold for right now.

However! The idea of adding an embedding compression system sounds very valuable, and it may make sense to add support for that in the existing memory systems and to apply that change set more broadly for the memory systems where it could be applied. I would assume this would only be redis and the local cache

Can you expand a bit more on why you want to use sql over the existing systems?

dschonholtz avatar May 16 '23 19:05 dschonholtz

My impression here is that there are several changes here that are not necessarily relevant to each other and they likely should be different PRs. Some of these would make sense to pull into master, others I am not so sure about.

If you could let me know if I am following correctly and if I am misunderstanding any pieces there correct me as needed. Also, if you could add what the value proposition is for these different pieces explicitly, that would be really helpful.

Here are the different pieces as I see it.

  1. Using SQLlite as a memory backend. I am not sure what the value of this is, it creates an additional dependency. It also is not easily queryable as the embeddings are not sorted per their similarity as far as I can see. Maybe sorting many different embeddings for different things by meta data?
  2. Compressing and storing embeddings for persistent memory with Zarr. This seems like it could be fairly useful. But the real value here, is unrelated to the rest of this PR. Effectively, I want to store memories from a previous run, and be able to load them, but I want them to take up minimal space between runs. That generally sounds useful.
  3. Using SKLearn nearest neighbor look up nearest embeddings. Again, I am not totally sure what the value proposition is here. This brings in the whole sklearn library and uses a different embeddings comparison strategy compared to the existing systems we have in place. I think it is very possible that we could support several strategies for embedding neighbor look ups, but that should generally be memory system agnostic. If we want to support various embedding comparison tools, we should probably add them as optional systems for each memory system so we can compare and use them for each memory system unless a system like pinecone for instance doesn't support the same look up systems in which case we would throw an error saying that look up system is not available.

dschonholtz avatar May 16 '23 19:05 dschonholtz

Like I said in my initial comment. The code looks pretty decent. I just want to make sure we don't add unnecessary bloat! Feel free to ping me on discord if you wanna chat about it

dschonholtz avatar May 16 '23 19:05 dschonholtz

Hey, I've marked this as don't merge until the Memory Fixes are in. Sorry to keep it on hold longer, just no real way to test/validate functionality until the fixes are in

ntindle avatar May 25 '23 15:05 ntindle