Removing an agent interferes with RandomActivation step method #302
Description
First of all, thanks so much for all the great work y'all've put into this. It's let me explore agent based modeling very quickly and easily.
I am implementing a model where agents have a finite lifespan, however, I noticed that my agents were persisting in the model (their AgentIDs were present in more Steps) than they should be for their lifespan. I tracked this down to a conflict between removing an agent and stepping through the model. If the agent's step() removes the agent from the model, this modifies the list of agents being iterated over in the scheduler's step() (starting at line 63 in schedule.py; step_breed() is called by step()):
def step_breed(self, breed):
'''
Shuffle order and run all agents of a given breed.
Args:
breed: Class object of the breed to run.
'''
agents = self.agents_by_breed[breed]
random.shuffle(agents)
for agent in agents: # <- If agent is removed, 'agents' is modified
agent.step()This leads to undesirable behavior (at least in the RandomActivation scheduler) where not all agents are called in each iteration if an agent is removed from the list. In the code below, all the agents should be present for only two steps maximum, however, some are present for more than that (the value varies by run presumably because of the shuffling of agents in the scheduler). Here's an example of what I mean:
from mesa.time import RandomActivation
from mesa.datacollection import DataCollector
from mesa import *
import numpy as np
class TestModel(Model):
def __init__(self, agent_lifetime = 1, n_agents = 100):
self.agent_lifetime = agent_lifetime
self.n_agents = n_agents
## keep track of the the remaining life of an agent and
## how many ticks it has seen
self.datacollector = DataCollector(
agent_reporters = {"remaining_life" : lambda a: a.remaining_life,
"steps" : lambda a: a.steps})
self.current_ID = 0
self.schedule = RandomActivation(self)
for _ in range(n_agents):
self.schedule.add(FiniteLifeAgent(self.get_next_ID(),
self.agent_lifetime))
def get_next_ID(self):
self.current_ID += 1
return self.current_ID
def step(self):
'''Add agents back to n_agents in each step'''
self.schedule.step()
self.datacollector.collect(self)
if len(self.schedule.agents) < self.n_agents:
for _ in range(self.n_agents - len(self.schedule.agents)):
self.schedule.add(FiniteLifeAgent(self.get_next_ID(),
self.agent_lifetime))
def run_model(self, step_count = 100):
for _ in range(step_count):
self.step()
class FiniteLifeAgent(Agent):
'''
An agent that is supposed to live for a finite number of ticks.
Also has a 10% chance of dying in each tick.
'''
def __init__(self, unique_id, lifetime):
self.unique_id = unique_id
self.remaining_life = lifetime
self.steps = 0
def step(self, model):
inactivated = self.inactivate(model)
self.steps += 1 # keep track of how many ticks are seen
if np.random.binomial(1,0.1) is not 0: # 10% chance of dying
model.schedule.remove(self)
def inactivate(self, model):
self.remaining_life -= 1
if self.remaining_life < 0:
model.schedule.remove(self)
return True
return False
model = TestModel()
model.run_model()
df = model.datacollector.get_agent_vars_dataframe()
df = df.reset_index()
# Get the total number of ticks an agent is present in.
lifetimes = df.groupby(["AgentID"]).Step.agg({"Step" : lambda x: len(x)})
# Varies between ~8 and 14 depending on the run.
print("Total number of ticks an agent is alive in should be 2. Actual value is: %i" % lifetimes.Step.max())
# actual value is 1 and 0, respectively, showing that all agents aren't called in each step of the scheduler
print("Total steps seen by agent should be 1. Actual value is: %i" % df.steps.max())
print("Minimum remaining life of agent should be 0. Actual value is: %i" % df.remaining_life.min())I have fixed this for my model by having the scheduler keep track of a dictionary of agents keyed by a unique integer that I stored in the agent's unique_id slot rather than a list of agents:
class BaseScheduler:
""" Simplest scheduler; activates agents one at a time, in the order
they were added.
Assumes that each agent added has a *step* method which takes no arguments.
(This is explicitly meant to replicate the scheduler in MASON).
"""
model = None
steps = 0
time = 0
agents = {}
def __init__(self, model):
""" Create a new, empty BaseScheduler. """
self.model = model
self.steps = 0
self.time = 0
self.agents = {}
def add(self, agent):
""" Add an Agent object to the schedule.
Args:
agent: An Agent to be added to the schedule. NOTE: The agent must
have a step() method.
"""
self.agents[agent.unique_id] = agent
def remove(self, agent):
""" Remove all instances of a given agent from the schedule.
Args:
agent: An agent object.
"""
del self.agents[agent.unique_id]
def step(self):
""" Execute the step of all the agents, one at a time. """
agent_keys = list(self.agents.keys())
for agent_key in agent_keys:
self.agents[agent_key].step(self.model)
self.steps += 1
self.time += 1
def get_agent_count(self):
""" Returns the current number of agents in the queue. """
return len(self.agents.keys())By iterating over the dictionary keys rather than the agents themselves, if an agent is removed the other agents are still stepped by the scheduler correctly.
I've made changes to the relevant classes in time.py in order to implement this, and they pass the unit tests. However, all classes that inherit the BaseScheduler class in example models would need to be examined and probably fixed. I also noticed that in some example models the unique_id slot is being used for other purposes than just identifying the agent. For example, in the WolfSheep model, it's being used to store the position of the agent. There could be other examples that would need updated, too.
I would be happy to fix up the WolfSheep model and submit a pull request if that would be helpful, but please advise this total newcomer to collaborative software how best to proceed!
best,
Colin