Memory module proposition - v2 #2744
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes introduce a new memory module within the Mesa experimental package. This update implements event management using a priority queue and weak references for handling simulation events. Additionally, it establishes a comprehensive memory management system with short-term and long-term memory for agents. New classes and methods support memory entry creation, retrieval, filtering, consolidation, serialization, and communication. Public entities such as Changes
Sequence Diagram(s)sequenceDiagram
participant Agent
participant Memory
participant STM as ShortTermMemory
participant LTM as LongTermMemory
Agent->>Memory: remember_short_term(entry)
Memory->>STM: add(entry)
STM-->>Memory: confirmation
Agent->>Memory: remember_long_term(entry)
Memory->>LTM: add(entry)
LTM-->>Memory: confirmation
Agent->>Memory: consolidate(entry)
Memory->>STM: remove(entry)
Memory->>LTM: add(entry)
sequenceDiagram
participant Simulation
participant EventManager as EventList
participant Event as SimulationEvent
Simulation->>EventManager: schedule event (with priority)
EventManager->>Event: initialize & manage event
Simulation->>EventManager: cancel event request
EventManager-->>Simulation: confirm cancellation
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
mesa/experimental/memory/__init__.py (2)
5-5: Fix the “dque” typo in the docstring.Line 5 refers to “dque data structure,” but the correct term is “deque.”
- (dque data structure) + (deque data structure)
23-25: Consider adding tests for the module exports.Static analysis shows these lines are uncovered by tests. If practical, add a lightweight test (e.g., verifying imports behave as expected) to maintain or increase coverage.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 23-23: mesa/experimental/memory/init.py#L23
Added line #L23 was not covered by tests
[warning] 25-25: mesa/experimental/memory/init.py#L25
Added line #L25 was not covered by testsmesa/experimental/memory/memory.py (3)
3-5: Correct the docstring grammar.“Then objective” should be “The objective” or “Its objective.”
- Then objective is to implement a very simple and efficient system... + The objective is to implement a very simple and efficient system...
24-24: Consider adding a__repr__method to ease debugging.When working with logs or interactive sessions, a customized
__repr__forMemoryEntrycan make debugging easier.class MemoryEntry: ... + def __repr__(self) -> str: + return f"<MemoryEntry type={self.entry_type}, step={self.entry_step}>"🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 24-24: mesa/experimental/memory/memory.py#L24
Added line #L24 was not covered by tests
19-287: Add unit tests for uncovered lines.Static analysis indicates these lines lack test coverage. Consider creating tests for core functionalities like
forget,get_by_type, and the memory consolidation flow.Would you like help generating sample test cases to increase overall coverage?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 19-21: mesa/experimental/memory/memory.py#L19-L21
Added lines #L19 - L21 were not covered by tests
[warning] 24-24: mesa/experimental/memory/memory.py#L24
Added line #L24 was not covered by tests
[warning] 27-27: mesa/experimental/memory/memory.py#L27
Added line #L27 was not covered by tests
[warning] 35-38: mesa/experimental/memory/memory.py#L35-L38
Added lines #L35 - L38 were not covered by tests
[warning] 40-40: mesa/experimental/memory/memory.py#L40
Added line #L40 was not covered by tests
[warning] 42-42: mesa/experimental/memory/memory.py#L42
Added line #L42 was not covered by tests
[warning] 49-50: mesa/experimental/memory/memory.py#L49-L50
Added lines #L49 - L50 were not covered by tests
[warning] 52-52: mesa/experimental/memory/memory.py#L52
Added line #L52 was not covered by tests
[warning] 58-58: mesa/experimental/memory/memory.py#L58
Added line #L58 was not covered by tests
[warning] 61-61: mesa/experimental/memory/memory.py#L61
Added line #L61 was not covered by tests
[warning] 67-67: mesa/experimental/memory/memory.py#L67
Added line #L67 was not covered by tests
[warning] 69-71: mesa/experimental/memory/memory.py#L69-L71
Added lines #L69 - L71 were not covered by tests
[warning] 73-73: mesa/experimental/memory/memory.py#L73
Added line #L73 was not covered by tests
[warning] 83-84: mesa/experimental/memory/memory.py#L83-L84
Added lines #L83 - L84 were not covered by tests
[warning] 86-87: mesa/experimental/memory/memory.py#L86-L87
Added lines #L86 - L87 were not covered by tests
[warning] 93-94: mesa/experimental/memory/memory.py#L93-L94
Added lines #L93 - L94 were not covered by tests
[warning] 96-96: mesa/experimental/memory/memory.py#L96
Added line #L96 was not covered by tests
[warning] 98-98: mesa/experimental/memory/memory.py#L98
Added line #L98 was not covered by tests
[warning] 100-100: mesa/experimental/memory/memory.py#L100
Added line #L100 was not covered by tests
[warning] 102-102: mesa/experimental/memory/memory.py#L102
Added line #L102 was not covered by tests
[warning] 104-104: mesa/experimental/memory/memory.py#L104
Added line #L104 was not covered by tests
[warning] 106-106: mesa/experimental/memory/memory.py#L106
Added line #L106 was not covered by tests
[warning] 108-108: mesa/experimental/memory/memory.py#L108
Added line #L108 was not covered by tests
[warning] 110-111: mesa/experimental/memory/memory.py#L110-L111
Added lines #L110 - L111 were not covered by tests
[warning] 113-113: mesa/experimental/memory/memory.py#L113
Added line #L113 was not covered by tests
[warning] 116-117: mesa/experimental/memory/memory.py#L116-L117
Added lines #L116 - L117 were not covered by tests
[warning] 119-119: mesa/experimental/memory/memory.py#L119
Added line #L119 was not covered by tests
[warning] 121-121: mesa/experimental/memory/memory.py#L121
Added line #L121 was not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mesa/experimental/memory/__init__.py(1 hunks)mesa/experimental/memory/memory.py(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
mesa/experimental/memory/__init__.py (1)
mesa/experimental/memory/memory.py (4)
LongTermMemory(148-204)Memory(207-286)MemoryEntry(24-58)ShortTermMemory(61-145)
🪛 GitHub Check: codecov/patch
mesa/experimental/memory/__init__.py
[warning] 23-23: mesa/experimental/memory/init.py#L23
Added line #L23 was not covered by tests
[warning] 25-25: mesa/experimental/memory/init.py#L25
Added line #L25 was not covered by tests
mesa/experimental/memory/memory.py
[warning] 19-21: mesa/experimental/memory/memory.py#L19-L21
Added lines #L19 - L21 were not covered by tests
[warning] 24-24: mesa/experimental/memory/memory.py#L24
Added line #L24 was not covered by tests
[warning] 27-27: mesa/experimental/memory/memory.py#L27
Added line #L27 was not covered by tests
[warning] 35-38: mesa/experimental/memory/memory.py#L35-L38
Added lines #L35 - L38 were not covered by tests
[warning] 40-40: mesa/experimental/memory/memory.py#L40
Added line #L40 was not covered by tests
[warning] 42-42: mesa/experimental/memory/memory.py#L42
Added line #L42 was not covered by tests
[warning] 49-50: mesa/experimental/memory/memory.py#L49-L50
Added lines #L49 - L50 were not covered by tests
[warning] 52-52: mesa/experimental/memory/memory.py#L52
Added line #L52 was not covered by tests
[warning] 58-58: mesa/experimental/memory/memory.py#L58
Added line #L58 was not covered by tests
[warning] 61-61: mesa/experimental/memory/memory.py#L61
Added line #L61 was not covered by tests
[warning] 67-67: mesa/experimental/memory/memory.py#L67
Added line #L67 was not covered by tests
[warning] 69-71: mesa/experimental/memory/memory.py#L69-L71
Added lines #L69 - L71 were not covered by tests
[warning] 73-73: mesa/experimental/memory/memory.py#L73
Added line #L73 was not covered by tests
[warning] 83-84: mesa/experimental/memory/memory.py#L83-L84
Added lines #L83 - L84 were not covered by tests
[warning] 86-87: mesa/experimental/memory/memory.py#L86-L87
Added lines #L86 - L87 were not covered by tests
[warning] 93-94: mesa/experimental/memory/memory.py#L93-L94
Added lines #L93 - L94 were not covered by tests
[warning] 96-96: mesa/experimental/memory/memory.py#L96
Added line #L96 was not covered by tests
[warning] 98-98: mesa/experimental/memory/memory.py#L98
Added line #L98 was not covered by tests
[warning] 100-100: mesa/experimental/memory/memory.py#L100
Added line #L100 was not covered by tests
[warning] 102-102: mesa/experimental/memory/memory.py#L102
Added line #L102 was not covered by tests
[warning] 104-104: mesa/experimental/memory/memory.py#L104
Added line #L104 was not covered by tests
[warning] 106-106: mesa/experimental/memory/memory.py#L106
Added line #L106 was not covered by tests
[warning] 108-108: mesa/experimental/memory/memory.py#L108
Added line #L108 was not covered by tests
[warning] 110-111: mesa/experimental/memory/memory.py#L110-L111
Added lines #L110 - L111 were not covered by tests
[warning] 113-113: mesa/experimental/memory/memory.py#L113
Added line #L113 was not covered by tests
[warning] 116-117: mesa/experimental/memory/memory.py#L116-L117
Added lines #L116 - L117 were not covered by tests
[warning] 119-119: mesa/experimental/memory/memory.py#L119
Added line #L119 was not covered by tests
[warning] 121-121: mesa/experimental/memory/memory.py#L121
Added line #L121 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (windows, 3.13)
- GitHub Check: run-benchmarks
🔇 Additional comments (2)
mesa/experimental/memory/__init__.py (1)
14-21: Confirm reference to event scheduling components.The docstring mentions
Priority,SimulationEvent, andEventList, but this file does not define or import them. Please verify if these references are part of another module or have yet to be implemented here.mesa/experimental/memory/memory.py (1)
73-80:❓ Verification inconclusive
Verify the repeated
modelparameter usage.Each
addmethod (in bothShortTermMemoryandLongTermMemory) plus theremember_short_term/remember_long_termmethods accept amodelparameter but also storeself.model. Confirm if passingmodelexplicitly is necessary or if usingself.modelsuffices. Removing the extra parameter could simplify the API.Also applies to: 159-166, 220-226
Action: Review and Clarify
modelParameter Usage
- In
mesa/experimental/memory/memory.py, theaddmethod (lines 73–80) accepts amodelparameter even though the instance already maintainsself.model. This pattern is repeated in the correspondingremember_short_termandremember_long_termmethods (lines 159–166 and 220–226).- Please verify whether the intent is to allow callers to override the stored
self.modelon a per-call basis. If not, refactoring the API to rely solely onself.modelcould simplify the implementation and reduce ambiguity.- Reconcile the design across both the short-term and long-term memory interfaces to ensure consistency.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 73-73: mesa/experimental/memory/memory.py#L73
Added line #L73 was not covered by tests
|
Performance benchmarks:
|
|
Thanks for the extensive work on this memory module. I had a similar question as @Corvince mentioned in the first PR (#2735 (comment)), about how this can be useful or necessary for all agent types in general. I understand that it is commonly used in LLM-based agents, but I'm trying to understand it from a more general perspective. For example, in our gis examples Rainfall and Urban Growth, agents are raindrops and land parcels. How do these inanimate objects have memories? Would it make sense to relate the current design of memory module, to the history of agents' attributes? Or if they are not the same, what's the difference between the two? There is an experimental feature for observables: #2291. Imagine I set |
|
I see what you mean @wang-boyu. I didn't really design it to fit in all scenarios the same way we don't necessarily us mesa signals for all simulations. The idea was to give the users a new tool and open a simple new range of experimentations with this. Examples I gave in answer to Corvince's feedback were :
These two examples use basic memory (and communication), but require the agents to have individual memories to make it interesting. The memory sharing feature also sets the base for communication without having to build another complicated module. I understand that it would be particularly useful with LLMs, but my opinion is that it can go beyond that. I also think that it could completely be compatible with the observables presented in #2291, I'll try to include this in the next commit for this PR ! I'll definitely try to make a model more complex than the foraging ants one to try to show more complex agent patterns with this memory module in the near future ! |
|
@colinfrisch I appreciate your continued work on this! I’m convinced that these use cases are relevant and a good fit for Mesa. I’m not convinced (yet) that we need a completely new module. What would really help me is two examples, in which you for each:
That would allow me to weigh the proportionality of the added complexity vs the user benefits. |
colinfrisch
force-pushed
the
memory_module
branch
2 times, most recently
from
4f27d88 to
8982579
Compare
|
Are you still working on this PR? Can I help move it forward? |
I'm currently working on a model using this memory module to compare it with the same model without. I should finish it by the end of the week. If it's not concluding, I'll post a recap of what can be learned from this PR and close it. |
|
Awesome, very curious! |
|
Back with some insights from building more models depending on agents storing and exchanging information. I built a virus vs antibody model to simulate an immune reaction declined as a confrontation between antibody agents and virus agents. The antibodies use a short term and a long term memory. I find the result quite interesting, and I'll open a PR to mesa_examples soon, but it's already available on my model repository if you want to take a look. The conclusion I had is that indeed, a complex memory module is not necessary for such models because memory in agents can simply be created in the init method, and they usually don't need complex retrieval features or very different types of storing : self.st_memory : deque = deque()
self.lt_memory : list = []These experimentations also helped me to reflect on the added value of this memory module. All in all, it has some, but it resides in :
Which are not necessarily needed in most basic agent simulations. Overall, it’s really what we are looking for in terms of memory LLM because they will need memories of very different types to be easily retrievable (text, reflexions, coordinates, states, etc.). Thanks again @EwoutH and @wang-boyu for your help and feedbacks, I'm closing this PR until further notice. |
|
Thanks for the detailed update! Have you discovered any feature that might be missing or worth adding to the AgentSet? |
@EwoutH I actually struggled quite a bit in terms of agent referencing (problems with agents still referenced even after being removed). I don't know yet if it's specific to the way I built my model ou to the way that weak referencing is handled in the AgentSet class, but I'll definitely look into it. Also, from what I understood, we have to create a new AgentSet each time that we want to create new agents in the simulation : for example, in the virus/antibody model, the agents can duplicate under certain conditions. There is already an |
|
Aside from that, you can assign any agent to any agentset whenever you want. Have you found bugs there or are missing features? Some (reproduceable) code examples would really help! |
Alright, definitely missed that. I had a not very clean but working code so I thought that was it. Definitely going to modify this, thanks for the help !
I'm going to refactor my code to hadle agents more cleanly in their AgentSets. If I still have issues with agent referencing that don't appear to be linked to my code, I will make a reproducible example. |
|
Just a couple of pointers which might be helpfult.
In my experience, with these 3 pointers, it is rare that you need to manage your own |
|
@EwoutH @quaquel thanks for the tips ! If fixed it directly into my program, but I don't know if it's something that could be handled directly in the mesa code (since the second agent doesn't exist, could there be a code snipped turning all the references pointing to him to I also built a small program to reproduce the error if you are interested. Don't hesitate to let me know if you have some feedback ! import os
import sys
from mesa import Model
from mesa.experimental.continuous_space import ContinuousSpace, ContinuousSpaceAgent
class MyAgent(ContinuousSpaceAgent):
def __init__(self, model, space):
super().__init__(model=model, space=space)
self.position = (0, 0)
self.target = None
def step(self):
# Find a target for agent 1
if self.target is None and len(self.model.agents_list) > 1:
self.target = [a for a in self.model.agents_list if a != self][0]
print(f"Agent {self.unique_id} targets {self.target.unique_id}")
# Show status and remove if first agent
if self.target:
print(f"Target space: {self.target.space}")
if self.unique_id == 1 and self.target.space is not None:
print(f"Removing target")
self.model.agents_list.remove(self.target)
self.target.remove()
# We don't set self.target = None - this causes the error
# Detect the error condition
if self.target and self.target.space is None:
print(f"Target exists but space is None ***********************")
class MyModel(Model):
def __init__(self):
super().__init__()
self.space = ContinuousSpace([[0, 10], [0, 10]], torus=True, random=self.random)
self.agents_list = []
# Create 2 agents
for _ in range(2):
agent = MyAgent(self, self.space)
self.agents_list.append(agent)
def step(self):
for agent in list(self.agents_list):
agent.step()
# Run the model
if __name__ == "__main__":
model = MyModel()
model.step() # First agent targets second agent |
|
thanks for the example. The simplest solution is to turn target into a weakref: Let me know if this is not clear enough. It took me a while to get my head around weakrefs when I first learned about them. as an aside, in the example you don't need the |
|
@quaquel (Thank you also for the model.agents tip - used it a few times while building but it slipped out of my mind) |
|
There is, as far as I know, no way of making refs weak automatically. However, it might be good to cover weakrefs and the importance of |
Alright ! I'm going to work on finding useful tips to write about it in the docs, and I'll open a PR if I do encounter interesting things. |
|
Ok, if I have some time I might also open a PR specifically on best practices regarding weakrefs, |
|
@EwoutH @quaquel if you plan to do a PR on weak referencing, it could even mention this example. I'd love to hear your feedbacks on it if you have time :) |
Memory PR upgrade
Based on the feedbacks from the previous PR (#2735) that I made on a proposition for a memory module in mesa (thanks again @EwoutH, @Corvince, @quaquel, if you also have insights for this one, im open to discussion !)
About the upgrade
Switching to a object format for optimized storage : MemoryEntry is now an object. One of the perks of using an object is not being obligated to have a uniqueID creation module as an object already has an intrinsic uniqueid
id(my_object). It saves storage space as well as lines of code.ShortTermMemory : more memory-efficient and reactive (efficient store and pop functionality)
get(),get_by_entry_type(), etc.LongTermMemory : more computational-efficient (efficient navigation)
Justification of memory transfering
tell_to()feature still here implemented directly in the memory (yay), renamed ascommunicate()to be more explicitI see this as "communicating in general is just a way to make a memory of something (like a sentence) and transmitting it to another being". I understand it's unusual, but if the memory module works well, it could mean opening agent communication without building other complex modules. I'm very open to discussion, so please tell me if you have any suggestions regarding this way of doing things !
A couple of examples of the structure of the new memory
Example usage
A little bit of performance analysis
The comparaison was done between the last memory structure (from #2735) and the new ShortTermMemory datastructure.
What I want to do next
search()method looking like this :Summary by CodeRabbit