remove_all_agents method added to model

[quaquel]

Closes #2393

At the moment, there is no convenience method for removing all agents from the model. Since model.agents returns a weakref agentset, doing operations on this won't work. It seems we need a model level method like model.remove_all_agents(), which would call agent.remove on each agent. It needs to run through agent.remove rather than just rebuild the model._agents datastructures to ensure that agents are also removed from the experimental cell spaces.

[github-actions]

Performance benchmarks:

Model	Size	Init time [95% CI]	Run time [95% CI]
BoltzmannWealth	small	🔵 +0.4% [-1.0%, +2.1%]	🔵 -0.8% [-1.0%, -0.6%]
BoltzmannWealth	large	🔵 +2.0% [+1.1%, +2.9%]	🔴 +6.5% [+4.4%, +8.4%]
Schelling	small	🔴 +3.6% [+3.2%, +4.0%]	🔵 +0.8% [+0.5%, +1.1%]
Schelling	large	🔵 +1.0% [+0.3%, +1.7%]	🔵 -5.0% [-7.8%, -1.5%]
WolfSheep	small	🔵 -1.4% [-1.7%, -1.1%]	🔵 -0.8% [-1.1%, -0.6%]
WolfSheep	large	🔵 -2.9% [-4.0%, -2.1%]	🟢 -6.9% [-8.8%, -5.0%]
BoidFlockers	small	🔵 +0.5% [+0.0%, +1.0%]	🔵 -0.1% [-0.8%, +0.5%]
BoidFlockers	large	🔵 -0.3% [-1.1%, +0.4%]	🔵 -1.2% [-1.9%, -0.6%]

[EwoutH]

Let's make it explicit what it does and call it remove_all_agents.

remove_agents we may want to use for something else also, like removing a certain AgentSet or list of Agents, or doing it conditionally, etc.

[quaquel]

I made the suggested name change.

[EwoutH]

Thanks!

Do we need to handle Agents being present in Spaces or other structures? (luckily they don't have schedulers anymore.) Or document something on how to handle this? And maybe test?

[quaquel]

This starts to feel like a round trip....

Tests were already included. I deliberately use agent.remove internally so it works for e.g., CellAgents. This is also stated in #2393. I will not bother with the existing spaces because they are a massive memory leak already and outside scope. The best solution for those is to subclass Agent.remove explicitly in your own custom agent class.

[EwoutH]

This starts to feel like a round trip....

I understand the feeling. I think there's a balance here. From my perspective, I can do a full, proper review end of day when I can take the time for it, and bundle anything in that single review.

Or I can jot down a quick thought immediately with the risk that I might have another one a bit later. In this case it were two thoughts within half an hour of the PR opening. That's still a lot faster than end-of-day (which I would also find a very acceptable timeline for a volunteer based project).

But let me know what you prefer.

We also do have to acknowledge that PR authors and PR reviewers have different roles and thus different responsibilities, but both have an essential role to play. I understand it can feel a bit asymmetrical because you have done most feature work last week, but I know exactly what it feels like. Part of it is avoidable, and we should strive to remove as much of that part as possible, but part of it is an essential pain of pushing things through at high velocity in a library committed to a long-term stable API.

Adding something to a standardized, user facing API just come with additional effort / costs compared to doing it yourself within the model.

I'm convinced if we keep appreciating both sides of this coin, whichever we occupy in a certain PR, we'll keep improving. I can't promise it will never be without any pain (for example, I do feel #2394), because sometimes a little pain a huge long term benefits. We can't prevent everything, but we can prevent a lot and should strive for that.

So do call things out, but always assume good intentions. If we need to discuss this further (in a call / face to face) let me know.

---

Tests were already included.

A bit of robustness would be nice here. Could you add some Agents to a custom AgentSet and Grid, and see if they are properly removed?

I deliberately use agent.remove internally so it works for e.g., CellAgents.

That's really nice, didn't know that!

This is also stated in #2393.

In my view, PR descriptions should be readable on their own. We have templates for these, let me check if I can add them to a menu, never got to that.

They don't have to be epistels, sometimes a single line per header could be enough.

Personally, I have largely automated this process using LLMs. For example, I find if you add the issue description, the code diff (add .diff to the PR URL (https://github.com/projectmesa/mesa/pull/2394.diff)) and optionally a few other important points/thought, LLMs are amazing in formatting and generating usage examples.

Prompt

Write a concise PR description using the issue and diff, following the PR template.

Removing all agents in the model
I am currently updating my teaching materials to MESA 3. One of my assignments is an evolutionary version of Axelrod's emergence of collaboration. However, this requires removing the entire population of agents at each tick, and creating the next generation. (I know it can also be implemented differently...). At the moment, there is no convenience method for removing all agents from the model. Since model.agents returns a weakref agentset, doing operations on this won't work. It seems we need a model level method like model.clear_agents(), which would call agent.remove on each agent. It needs to run through agent.remove rather than just rebuild the model._agents datastructures to ensure that agents are also removed from the experimental cell spaces.

diff --git a/mesa/model.py b/mesa/model.py
index ac59cc1f1f9..ac6d5d3215c 100644
--- a/mesa/model.py
+++ b/mesa/model.py
@@ -276,3 +276,9 @@ def initialize_data_collector(
         )
         # Collect data for the first time during initialization.
         self.datacollector.collect(self)
+
+    def remove_all_agents(self):
+        """Remove all agents from the model."""
+        # we need to wrap keys in a list to avoid a RunTimeError: dictionary changed size during iteration
+        for agent in list(self._agents.keys()):
+            agent.remove()
diff --git a/tests/test_model.py b/tests/test_model.py
index 7c343e46357..ff1fe297b6a 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -93,3 +93,18 @@ class Sheep(Agent):
     assert model.agents_by_type[Wolf] == AgentSet([wolf], model)
     assert model.agents_by_type[Sheep] == AgentSet([sheep], model)
     assert len(model.agents_by_type) == 2
+
+
+def test_agent_remove():
+    """Test removing all agents from the model."""
+
+    class TestAgent(Agent):
+        pass
+
+    model = Model()
+    for _ in range(100):
+        TestAgent(model)
+    assert len(model.agents) == 100
+
+    model.remove_all_agents()
+    assert len(model.agents) == 0

PR template

## Summary
<!-- Provide a concise summary of the feature and its purpose. -->

## Motive
<!-- Explain the reasoning behind this feature. Include details on the problem it addresses or the enhancement it provides. -->

## Implementation
<!-- Describe how the feature was implemented. Include details on the approach taken, important decisions made, and code changes. -->

## Usage Examples
<!-- Provide code snippets or examples demonstrating how to use the new feature. Highlight key scenarios where this feature will be beneficial.

If you're modifying the visualisation, add before/after screenshots. -->

## Additional Notes
<!-- Add any additional information that may be relevant for the reviewers, such as potential side effects, dependencies, or related work. -->

Response

Here's a concise PR description following the provided template:

Summary

Add a remove_all_agents() method to the Model class to facilitate removing all agents from a model in a single operation.

Motive

This feature addresses the need to efficiently remove all agents from a model, particularly useful in scenarios like evolutionary simulations where the entire population needs to be replaced each tick. Currently, there's no convenient way to perform this operation, especially considering the weakref nature of model.agents.

Implementation

• Added remove_all_agents() method to the Model class in mesa/model.py.
• The method iterates through all agents using self._agents.keys() wrapped in a list to avoid runtime errors due to dictionary size changes.
• Each agent is removed using the agent.remove() method to ensure proper cleanup, including removal from experimental cell spaces.
• Added a unit test in tests/test_model.py to verify the functionality of the new method.

Usage Examples

model = MyModel()
# ... add agents and run simulation ...
model.remove_all_agents()  # Removes all agents from the model
assert len(model.agents) == 0

Additional Notes

This method ensures that agents are properly removed using their remove() method, maintaining consistency with other agent removal operations in Mesa.

Of course you have to review and where necessary edit these, and never blindly copy, but I find they are often 90%+ of the case good, and often I only edit a few words or a add a sentence somewhere. And writing them yourself also has some charm, so sometimes I also do that.

Good PR descriptions are helpful for reviewers, but also historical documentation and especially useful in a public-facing library, where users might want to know why a feature is designed like the way it is.

I will not bother with the existing spaces because they are a massive memory leak already and outside scope.

Okay, in this specific case I can get behind this, but maybe add a line of documentation that Agents are removed from the cell space automatically, but not from the current spaces.

The best solution for those is to subclass Agent.remove explicitly in your own custom agent class.

I was also thinking in that direction. Would a simple test case that shows this be possible?

[quaquel]

A bit of robustness would be nice here. Could you add some Agents to a custom AgentSet and Grid, and see if they are properly removed?

I don't understand what you are proposing. This adds a method to the model class so why test other non-Model class related things? That is more of a form of integration testing. Moreover, mesa.space.Grid and its subclasses won't work because its not covered in Agent.remove (and cannot be covered there because agents don't know about mesa.space classes.

Okay, in this specific case I can get behind this, but maybe add a line of documentation that Agents are removed from the cell space automatically, but not from the current spaces.

I was also thinking in that direction. Would a simple test case that shows this be possible?

No. What would make more sense is improving the documentation of Agent.remove. If we expand this documentation you introduce potential unwanted dependencies in the docs between the agent class and the model class. Therefore adding tests in test_model for testing agent.remove also appears strange to me.

[EwoutH]

From my view the difference is when removing an Agent in a loop or one by one, you often have code around it to handle other removals, like the space. With remove_all_agents, users might think, "oh, that's convenient, a proper build in function!" and then that's not anymore the case.

In my opinion there should be something somewhere for people to find to resolve their error when inevitably they think remove all agents from a model that has a Space. It doesn't need to handle it perfectly all the time be default, but a piece of documentation or testing should be there. That's the cost of doing business in a public API.

Once we have deprecated the spaces formally, this will be a different story. And remove_all_agents could also be just a function of a custom user model, since their really isn't that much added value to add it to Mesa if it doesn't handle supported (maintenance or not) edge cases properly.

Of course any maintainers might have different opinions about any of this, and I'm happy to follow a majority.

[quaquel]

From my view the difference is when removing an Agent in a loop or one by one, you often have code around it to handle other removals, like the space. With remove_all_agents, users might think, "oh, that's convenient, a proper build in function!" and then that's not anymore the case.

Fair enough. I have added something on this in Agent.remove and Model.remove_all_agents.

[EwoutH]

This covers it well, thanks!

Could you make the PR description readable on it's own?