From 4a1102fb987642700c3b902ab49a168cf1b29ca2 Mon Sep 17 00:00:00 2001
From: Ewout ter Hoeven <E.M.terHoeven@student.tudelft.nl>
Date: Fri, 26 Jan 2024 18:01:35 +0100
Subject: [PATCH] Fix AgentSet inplace shuffle (and thus RandomActivation), add
 tests (#2007)

* tests: Add test to check if RandomActivation is not sequential

Adds a test that checks if the RandomActivation doesn't trigger agents in a sequential order.

In theory this could give false positives (a test passing when it shouldn't, but that chance is around ~0.1^18).

* fix for RandomActivation bug

fixes #2006

* add agentset.shuffle unittest

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* ruff fix

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add agent to model.agent correctly even if super was not called

fix for the second issue in #2006. If super is not present, we create the data structure but forget to add the agent to it. This is just a backward compatibility fix.

* test: Shuffle more agents to prevent false negatives

No the chance on a false negative is one in 12! instead of 4! (40 million instead of 24)

---------

Co-authored-by: Jan Kwakkel <j.h.kwakkel@tudelft.nl>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
---
 mesa/agent.py       |  1 +
 mesa/time.py        |  4 ++--
 tests/test_agent.py | 13 +++++++++++++
 tests/test_time.py  | 22 ++++++++++++++++++++++
 4 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/mesa/agent.py b/mesa/agent.py
index fcf643d3289..6155284ba42 100644
--- a/mesa/agent.py
+++ b/mesa/agent.py
@@ -54,6 +54,7 @@ def __init__(self, unique_id: int, model: Model) -> None:
         except AttributeError:
             # model super has not been called
             self.model.agents_ = defaultdict(dict)
+            self.model.agents_[type(self)][self] = None
             self.model.agentset_experimental_warning_given = False
 
             warnings.warn(
diff --git a/mesa/time.py b/mesa/time.py
index 3dcd1708f84..50d564be551 100644
--- a/mesa/time.py
+++ b/mesa/time.py
@@ -150,8 +150,8 @@ def get_agent_keys(self, shuffle: bool = False) -> list[int]:
 
     def do_each(self, method, shuffle=False):
         if shuffle:
-            self.agents.shuffle(inplace=True)
-        self.agents.do(method)
+            self._agents.shuffle(inplace=True)
+        self._agents.do(method)
 
 
 class RandomActivation(BaseScheduler):
diff --git a/tests/test_agent.py b/tests/test_agent.py
index 5861038d793..60a0eec15fa 100644
--- a/tests/test_agent.py
+++ b/tests/test_agent.py
@@ -251,3 +251,16 @@ def test_agentset_select_by_type():
     # Test with no type specified (should select all agents)
     all_agents = agentset.select()
     assert len(all_agents) == len(mixed_agents)
+
+
+def test_agentset_shuffle():
+    model = Model()
+    test_agents = [TestAgent(model.next_id(), model) for _ in range(12)]
+
+    agentset = AgentSet(test_agents, model=model)
+    agentset = agentset.shuffle()
+    assert not all(a1 == a2 for a1, a2 in zip(test_agents, agentset))
+
+    agentset = AgentSet(test_agents, model=model)
+    agentset.shuffle(inplace=True)
+    assert not all(a1 == a2 for a1, a2 in zip(test_agents, agentset))
diff --git a/tests/test_time.py b/tests/test_time.py
index 3cfc2f35078..53967d1a8ef 100644
--- a/tests/test_time.py
+++ b/tests/test_time.py
@@ -224,6 +224,28 @@ def test_get_agent_keys(self):
         agent_ids = {agent.unique_id for agent in model.agents}
         assert all(entry in agent_ids for entry in keys)
 
+    def test_not_sequential(self):
+        model = MockModel(activation=RANDOM)
+        # Create 10 agents
+        for _ in range(10):
+            model.schedule.add(MockAgent(model.next_id(), model))
+        # Run 3 steps
+        for _ in range(3):
+            model.step()
+        # Filter out non-integer elements from the log
+        filtered_log = [item for item in model.log if isinstance(item, int)]
+
+        # Check that there are no 18 consecutive agents id's in the filtered log
+        total_agents = 10
+        assert not any(
+            all(
+                (filtered_log[(i + j) % total_agents] - filtered_log[i]) % total_agents
+                == j % total_agents
+                for j in range(18)
+            )
+            for i in range(len(filtered_log))
+        ), f"Agents are activated sequentially:\n{filtered_log}"
+
 
 class TestSimultaneousActivation(TestCase):
     """