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

Use Chroma to augment and make LocalCache memory/embeddings pluggable #1027

Closed

Conversation

swyxio
Copy link

@swyxio swyxio commented Apr 13, 2023

Background

The LocalCache memory backend implemented in #372 used a handrolled implementation of a storage engine, and the embedding strategy was pinned to text-embedding-ada-002 which we may want to change/does cost money/latency

Changes

  • reimplemented LocalCache using Chroma - which also runs either in-memory or persisted, but is well tested and managed.
  • made embeddings swappable to ANY openai embedding model - and uses the Chroma default SentenceTransformers which costs no money

Documentation

in README

Test Plan

testing it locally

no new tests needed since this is a strict upgrade of LocalCache.

PR Quality Checklist

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

@swyxio swyxio closed this Apr 13, 2023
@swyxio swyxio reopened this Apr 13, 2023
@swyxio
Copy link
Author

swyxio commented Apr 13, 2023

its ready for review :)

@aosan
Copy link

aosan commented Apr 13, 2023

great addition @sw-yx ! I hope it gets merged soon.

nponeccop
nponeccop previously approved these changes Apr 13, 2023
@nponeccop
Copy link
Contributor

@sw-yx There are conflicts

@swyxio
Copy link
Author

swyxio commented Apr 13, 2023

whaaat i just made this PR yesterday and there are conflicts haha. will fix!

@nponeccop
Copy link
Contributor

We move fast, new conflicts can happen every 5 minutes here :)

@swyxio
Copy link
Author

swyxio commented Apr 13, 2023

@nponeccop i think i fixed the conflict!

nponeccop
nponeccop previously approved these changes Apr 13, 2023
@swyxio swyxio closed this Apr 14, 2023
@swyxio swyxio force-pushed the swyx/extendLocalCache branch from 4c8aaa5 to 1b3f82e Compare April 14, 2023 21:51
@swyxio
Copy link
Author

swyxio commented Apr 14, 2023

re-fixed new conflicts! @nponeccop what typically happens next?

@nponeccop nponeccop mentioned this pull request Apr 14, 2023
1 task
nponeccop
nponeccop previously approved these changes Apr 14, 2023
@nponeccop
Copy link
Contributor

@sw-yx There are conflicts now

@swyxio
Copy link
Author

swyxio commented Apr 15, 2023

@nponeccop as far as i can tell i fixed the conflicts as of commit a6928c2. i've just updated my fork to current master but still not seeing any conflicts as of right now!

@nponeccop
Copy link
Contributor

@sw-yx There are both conflicts and CI failure now

@github-actions github-actions bot added size/xs and removed size/xl labels Apr 25, 2023
@swyxio swyxio reopened this Apr 25, 2023
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added size/xl and removed conflicts Automatically applied to PRs with merge conflicts size/xs labels Apr 25, 2023
@github-actions
Copy link
Contributor

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@swyxio
Copy link
Author

swyxio commented Apr 25, 2023

This PR exceeds the recommended size of 1000 lines.

no, it only has ~100 lines changed...?

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Apr 25, 2023
@github-actions
Copy link
Contributor

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

@swyxio swyxio closed this Apr 26, 2023
@swyxio swyxio force-pushed the swyx/extendLocalCache branch from 009abd7 to d753793 Compare April 26, 2023 03:54
@github-actions github-actions bot added size/xs and removed size/xl labels Apr 26, 2023
@swyxio swyxio reopened this Apr 26, 2023
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added size/l conflicts Automatically applied to PRs with merge conflicts and removed conflicts Automatically applied to PRs with merge conflicts size/xs labels Apr 26, 2023
@github-actions
Copy link
Contributor

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

@p-i-
Copy link
Contributor

p-i- commented May 5, 2023

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

@Pwuts
Copy link
Member

Pwuts commented Jun 14, 2023

First of all: sorry for leaving this unresolved for so long. This is an interesting PR and unfortunately I'm only seeing it now for the first time.

Since this PR, we have rebuilt the memory storage system (#4208) and removed all third-party memory stores, to make room for redeveloping the entire memory system without having to worry about maintaining multiple memory stores.
Once we have a PoC of Auto-GPT working with vector memory again, I'd be interested to see a comparison with a simpler implementation using Chroma.

For now, I can only close this, as the PR is based on a very much outdated version of the codebase and because we want to have only one memory backend until the memory system is in better shape. We would still be happy to have your involvement; feel free to join in on the action via discord!

@Pwuts Pwuts closed this Jun 14, 2023
@Pwuts Pwuts changed the title augment and make LocalCache memory/embeddings pluggable augment and make LocalCache memory/embeddings pluggable with Chroma Jun 14, 2023
@Pwuts Pwuts changed the title augment and make LocalCache memory/embeddings pluggable with Chroma Use Chroma to augment and make LocalCache memory/embeddings pluggable Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts Automatically applied to PRs with merge conflicts function: memory Obsolete? size/l
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants