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

Fix Milvus as a long-term memory backend. #1658

Merged
merged 2 commits into from
Apr 15, 2023

Conversation

wangxuqi
Copy link
Contributor

Hi, there
As I mentioned in 1515, There are some bugs with Milvus long-term memory. This pull request is try to refactor and fix it.

Background

The enable milvus as memory backend leads some bugs and the code even not worked.

  • incorrect package import path
  • wrong memory implement directory
  • wrong interface implement
  • no tests

Changes

  • fix import path of milvus.py.
  • move milvus.py from script/memory to autogpt/memory/, put all memory implements together.
  • add unit and integration tests.
  • implement clear function correctly.

Documentation

As I explained above.

Test Plan

All unit test and integration test passed

============================= test session starts ==============================
collecting ... collected 5 items

milvus_memory_test.py::TestMilvusMemory::test_add 
milvus_memory_test.py::TestMilvusMemory::test_clear 
milvus_memory_test.py::TestMilvusMemory::test_get 
milvus_memory_test.py::TestMilvusMemory::test_get_relevant 
milvus_memory_test.py::TestMilvusMemory::test_get_stats 

============================== 5 passed in 21.75s ==============================

And I've also tested with the following instructions:

Name:  Summarize today's leading news
Role:  an AI designed to autonomously develop and run businesses with the
Goals: ["Visit a website which provides today's news.", 'Summarize the articles and write the results to text files', 'Terminate']
Continue (y/n): y
Using memory of type:  MilvusMemory
Using Browser:  chrome
 THOUGHTS:  I suggest browsing some news websites to get the latest headlines, then summarizing the articles and saving the results to a text file for later reference.

you can see it print "Using memory of type: MilvusMemory"

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

@wangxuqi
Copy link
Contributor Author

This pull request may conflict with another one you have approved. If it does, I will fix it.

richbeales
richbeales previously approved these changes Apr 15, 2023
@richbeales
Copy link
Contributor

Run flake8 autogpt/ tests/ --select E303,W293,W291,W292,E305,E231,E302
tests/milvus_memory_test.py:64:20: W292 no newline at end of file

p-i-
p-i- previously approved these changes Apr 15, 2023
@p-i-
Copy link
Contributor

p-i- commented Apr 15, 2023

tests/milvus_memory_test.py:64:20: W292 no newline at end of file

Can you fix?

@p-i- p-i- dismissed stale reviews from richbeales and themself via a1d2010 April 15, 2023 18:51
@richbeales richbeales merged commit 1c12a84 into Significant-Gravitas:master Apr 15, 2023
sindlinger pushed a commit to Orgsindlinger/Auto-GPT-WebUI that referenced this pull request Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants