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

[Draft, Feedback Needed] Memory in AgentChat #4438

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

victordibia
Copy link
Collaborator

@victordibia victordibia commented Dec 1, 2024

Memory for AgentChat Agents

It would be useful to have some notion of memory, and the ability to attach memory to an agent.
Right now the AssitantAgent can take on tools.

agent = Agent(model=model, tools=[] )

Some use cases often benefit from being able to retrieve memory just in time, add to the prompt before responding (RAG etc).

agent = Agent(model=model, tools=[], memory=[])

This PR implements

  • Memory Protocol
  • ListMemory - simple memory based on a list and basic similarity matching.
  • ChromaDBMemory - implemented using ChromaDB with similar expected behaviour for other vectordb offerins such as pinecone, scann, faiss, mongodb etc. (this impl is added more as an example and might be removed and added somewhere else e.g., in autogen_ext or some 3rd party repo)

Memory Behaviour

Memory protocol that devs can overload.

@runtime_checkable
class Memory(Protocol):
    """Protocol defining the interface for memory implementations."""

    @property
    def name(self) -> str | None:
        """The name of this memory implementation."""
        ...

    @property
    def config(self) -> BaseMemoryConfig:
        """The configuration for this memory implementation."""
        ...

    async def query(
        self,
        query: Union[str, Image, List[Union[str, Image]]],
        cancellation_token: CancellationToken | None = None,
        **kwargs: Any
    ) -> List[MemoryQueryResult]:
        """
        Query the memory store and return relevant entries.

        Args:
            query: Text, image or multimodal query
            cancellation_token: Optional token to cancel operation
            **kwargs: Additional implementation-specific parameters

        Returns:
            List of memory entries with relevance scores
        """
        ...

    async def add(
        self,
        entry: MemoryEntry,
        cancellation_token: CancellationToken | None = None
    ) -> None:
        """
        Add a new entry to memory.

        Args:
            entry: The memory entry to add
            cancellation_token: Optional token to cancel operation
        """
        ...

    async def clear(self) -> None:
        """Clear all entries from memory."""
        ...

    async def cleanup(self) -> None:
        """Clean up any resources used by the memory implementation."""
        ...
   

Integrating with AssistantAgent

Perhaps a big change with this PR is how AssistantAgent is extended to use memory.

  • AssistantAgent will try to query memory using message[-1] in on_messages_stream (if TextMessage, or MultiModalMessage), returned result is appended to model_context
  • The implementation AssistantAgent impl above focuses on memory.query and adds that JIT to the agent context. It does not concern itself much with how stuff is added to memory - reason being that his can be heavily usecase driven. It is expected that the developer will run memory.add outside of agent logic .
  • Developers can implement their own custom memory classes by implementing the Memory protocol.

Example Implementation

Example notebook highlighting these.

from autogen_agentchat.memory._base_memory import MemoryEntry
from autogen_agentchat.memory._chroma_memory import ChromaMemory, ChromaMemoryConfig

 
# Initialize memory
chroma_memory = ChromaMemory(
    name="travel_memory",
    config=ChromaMemoryConfig(
        collection_name="travel_facts",
        # Configure number of results to return instead of similarity threshold
        k=1  
    )
)
# Add some travel-related memories
await chroma_memory.add(MemoryEntry(
    content="Paris is known for the Eiffel Tower and amazing cuisine.",
    source="travel_guide"
))

await chroma_memory.add(MemoryEntry(
    content="The most important thing about tokyo is that it has the world's busiest railway station - Shinjuku Station.",
    source="travel_facts"
))

# Create agent with memory
agent = AssistantAgent(
    name="travel_agent",
    model_client=OpenAIChatCompletionClient(
        model="gpt-4o",
        # api_key="your_api_key"
    ),
    memory=chroma_memory,
    system_message="You are a travel expert"
)

agent_team = RoundRobinGroupChat([agent], termination_condition = MaxMessageTermination(max_messages=2))
stream = agent_team.run_stream(task="Tell me the most important thing about Tokyo.")
await Console(stream);
---------- user ----------
Tell me the most important thing about Tokyo.
---------- travel_agent ----------
One of the most important aspects of Tokyo is that it has the world's busiest railway station, Shinjuku Station. This station serves as a major hub for transportation, with millions of commuters and travelers passing through its complex network of train lines each day. It highlights Tokyo's status as a bustling metropolis with an advanced public transportation system.
[Prompt tokens: 72, Completion tokens: 66]
---------- Summary ----------
Number of messages: 2
Finish reason: Maximum number of messages 2 reached, current message count: 2
Total prompt tokens: 72
Total completion tokens: 66
Duration: 1.47 seconds

Related issue number

Closes #4039, #4648

TBD

  • Finalize design
  • Add tests

Checks

Open Questions

  • Should memory be a list or simple? memory = [chroma_memory] or memory = chroma_memory . E.g, should an agent had the opportunity to "dip" into several memory banks?


async def query(
self,
query: Union[str, Image, List[Union[str, Image]]],
Copy link
Collaborator

Choose a reason for hiding this comment

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

query also could benefit of mimetypes

if not isinstance(last_message, TextMessage) and not isinstance(last_message, MultiModalMessage):
raise ValueError(
"Memory query failed: Last message must be a text message or multimodal message.")
results: List[MemoryQueryResult] = await self._memory.query(messages[-1].content, cancellation_token=cancellation_token)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is saying that the default memory implementation is RAG with the last message.

I feel that using the last message for the RAG is reasonable but restrictive, could we just pass messages and have the memory query decide?

A high level comment that I'm not sure whether to call this memory or datastore/database.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@husseinmozannar , I agree about passing the entire context and let the query method decide.

 query: ContentItem | List[ContentItem] 

I am flexible on naming.
I like Memory because it connotes "just in time" retrieval/recall of content relevant to a step (in a task) an agent is about to take. Memory also gives a sense of what is stored inside the memory - in this case it really should be content relevant to task completion (not just anything that can be in a database).

@afourney
Copy link
Member

afourney commented Dec 5, 2024

I agree with @husseinmozannar that this is a reasonable and very clean implementation of this idea -- if perhaps a little restrictive. I like the idea of passing the entire context (or perhaps even state!) to the query engine. It's also worth thinking if we can somehow parameterize how the memory is added to the context at the time of the inference. E.g., this implementation adds memory right after the system prompt, and without any explanation or preamble. Other implementations are also reasonable. For example you could introduce memory with something like: "As you work through the user's request, the following snippets may, or may not, be helpful:" You could decide to include memory as the second-to-last message, or the last message (rather than the second). In AutoGen 0.2, we had the idea of context transformers. I wonder if something similar could work here.

@ekzhu
Copy link
Collaborator

ekzhu commented Dec 11, 2024

I second @afourney and @husseinmozannar's suggestion. I think the query method forces the caller of the memory (e.g., AssistantAgent) make an upfront choice on how memory is used to added to the context.

How about let the memory protocol provide a transform method that takes a model context (i.e., a list of LLMMessage, tool calls, etc.) and returns a transformed model context that can be sent to the model client directly. This way the caller of the memory module doesn't need to make an opinionated decision on how to query and how to use the result, rather, we can leave this decision to the memory module itself, and caller of AssistantAgent can choose from a preset or customize this from application.

There is a ModelContext module in the Core API that is barely used, perhaps we can refine that one and make it work side-by-side with the memory protocol

@gagb
Copy link
Collaborator

gagb commented Dec 18, 2024

Chatted with @victordibia API is nice and clean and I agree with its usefulness.

It would be useful to have following somewhere in the repo but not in the base protocol

  • example of memory related events being raised for observability
  • example of agent selectively calling .pop on the memory
  • example of agent selectively calling .add on the memory -- replicates memory feature in ChatGPT UI.
  • a full fledged RAG agent implemented using this protocol. I would like to be able to add AutoGen repo to it and ask questions.

@gagb gagb mentioned this pull request Dec 19, 2024
3 tasks
@lspinheiro
Copy link
Collaborator

Chatted with @victordibia API is nice and clean and I agree with its usefulness.

It would be useful to have following somewhere in the repo but not in the base protocol

  • example of memory related events being raised for observability
  • example of agent selectively calling .pop on the memory
  • example of agent selectively calling .add on the memory -- replicates memory feature in ChatGPT UI.
  • a full fledged RAG agent implemented using this protocol. I would like to be able to add AutoGen repo to it and ask questions.

Since

Chatted with @victordibia API is nice and clean and I agree with its usefulness.

It would be useful to have following somewhere in the repo but not in the base protocol

  • example of memory related events being raised for observability
  • example of agent selectively calling .pop on the memory
  • example of agent selectively calling .add on the memory -- replicates memory feature in ChatGPT UI.
  • a full fledged RAG agent implemented using this protocol. I would like to be able to add AutoGen repo to it and ask questions.

Adding my 2 cents here. I think would be interesting to have a lower-level abstraction for storage and information types which MimeType and MemoryContent are derived from. There may be some differences between knowledge-base retrieval vs memory retrieval that may be useful to consider when creating these abstractions.

I think it could be useful to think how memory uses storage and have chroma as a storage implementation that some VectorEmbeddingMemory uses and then users can easily swap whatever vector database they want to use. Then the storage abstraction can possibly be adapted to some knowledge-base retrievers we decide to implement. I think most other agentic frameworks such as semantic kernel and langchain also have abstractions at the storage layer and it may be easier for us to create adapters in this way.

@victordibia
Copy link
Collaborator Author

@lspinheiro ,
The AgentChat framework will likely only have the Memory protocol, developers should overload it to implement whatever vector, graph or any other type of Just in time memory they need for their agent.

I think would be interesting to have a lower-level abstraction for storage and information types which MimeType and MemoryContent are derived from

Good idea, can you propose some concrete examples?

I think it could be useful to think how memory uses storage and have chroma as a storage implementation that some VectorEmbeddingMemory uses and then users can easily swap whatever vector database they want to use.

I think I understand your comment here ie. that VectorEmbeddingMemory is a general enough case that we should explore some standardized implemetation that enables easily switching out various standard dbs. One thing to note is that the apis for this DBs are so different that there will still be quite a bit of code written specifically for each. That being said, perhaps we can get the base Memory protocol done in this PR and then open an new issue for designing something for VectorEmbeddingMemory

@ekzhu
Copy link
Collaborator

ekzhu commented Dec 20, 2024

Agree with @victordibia here, let's focus on the memory protocol first before worrying about the implementation level stuff.

Furthermore, I would argue that we should be careful not to introduce too many abstractions.

open an new issue for designing something for VectorEmbeddingMemory

We should take a look at Semantic Kernel's vector memory abstraction and consider adopt that or duck type it.

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.

Memory Interface for AgentChat agents
7 participants