Repository: projectmesa/mesa
Pull request #2478 (2024-11-09 21:09:54)

Bugfixes to DEVS
This isolates the changes to DEVS from #2470 into their own stand-alone PR.

While working on #2479, I discovered a few subtle bugs in `experimental.devs` and the `wolf-sheep` benchmark model
1. setup cleared the event list, but a model (e.g., wolf sheep) might have already scheduled events. Simulators now raises exceptions when (a) setup is called while either time != starttime or when events have already been scheduled; (b) any of the run methods will raise an exception if setup has not yet been called. 
2. A knock-on consequence of point 1 is that `simulator.setup` should now be called inside the `model.__init__` if the `__init__` triggers any event scheduling. For example, for wolf-sheep this is the case because all grass patches that are not fully grown will schedule their moment of being fully regrown. 
3. with #2223 and the associated discussion, there was now a bug in `ABMSimulator`, which scheduled the first step on $t=0$, while this should be $t=1$.


github-actions[bot]: Performance benchmarks:

| Model           | Size   | Init time [95% CI]      | Run time [95% CI]          |
|-----------------|--------|-------------------------|----------------------------|
| BoltzmannWealth | small  | 🔵 -0.5% [-0.9%, -0.1%] | 🔵 +0.2% [+0.1%, +0.3%]    |
| BoltzmannWealth | large  | 🔵 +0.2% [-0.3%, +0.6%] | 🔵 +1.0% [+0.6%, +1.4%]    |
| Schelling       | small  | 🔵 -0.0% [-0.2%, +0.2%] | 🔵 +0.6% [+0.5%, +0.8%]    |
| Schelling       | large  | 🔵 -0.3% [-0.6%, +0.0%] | 🔵 +0.3% [-0.1%, +0.7%]    |
| WolfSheep       | small  | 🔵 +0.7% [+0.3%, +1.1%] | 🔴 +78.2% [+75.4%, +80.8%] |
| WolfSheep       | large  | 🔵 -1.3% [-1.6%, -1.0%] | 🔴 +55.1% [+53.0%, +57.2%] |
| BoidFlockers    | small  | 🔵 +1.5% [+0.9%, +2.0%] | 🔵 +2.7% [+1.9%, +3.6%]    |
| BoidFlockers    | large  | 🔵 +0.9% [+0.5%, +1.4%] | 🔵 +0.9% [+0.3%, +1.5%]    |

EwoutH: Thanks, really appreciated. Generally, it looks good. Let's wait for the benchmarks to confirm performance.

Unrelated, but to confirm, the best practice is currently to define the `simulator.setup(model)` insode the init of the Model class, so `self.simulator.setup(self)`?
github-actions[bot]: Performance benchmarks:

| Model           | Size   | Init time [95% CI]      | Run time [95% CI]          |
|-----------------|--------|-------------------------|----------------------------|
| BoltzmannWealth | small  | 🔵 +1.2% [+0.3%, +2.0%] | 🔵 +1.7% [+1.5%, +1.9%]    |
| BoltzmannWealth | large  | 🔵 +2.0% [+1.4%, +2.7%] | 🔵 +0.7% [+0.1%, +1.3%]    |
| Schelling       | small  | 🔵 +3.3% [+2.9%, +3.6%] | 🔵 +1.7% [+1.5%, +1.9%]    |
| Schelling       | large  | 🔵 +2.9% [+2.5%, +3.3%] | 🔵 +0.9% [+0.2%, +1.5%]    |
| WolfSheep       | small  | 🔵 +0.9% [+0.5%, +1.3%] | 🔴 +74.6% [+71.8%, +77.2%] |
| WolfSheep       | large  | 🔵 +0.4% [-0.1%, +0.9%] | 🔴 +52.6% [+49.3%, +55.9%] |
| BoidFlockers    | small  | 🔵 +0.4% [-0.2%, +0.9%] | 🔵 -2.2% [-2.8%, -1.6%]    |
| BoidFlockers    | large  | 🔵 -0.2% [-1.0%, +0.9%] | 🔵 -2.3% [-3.0%, -1.5%]    |

EwoutH: Unfortunately, the performance degradation looks reproducible and significant.

Any idea what causing it?
quaquel: > Unrelated, but to confirm, the best practice is currently to define the simulator.setup(model) insode the init of the Model class, so self.simulator.setup(self)? 

Yes. If setup is called after events have been scheduled, you get an exception. So if `__init__` results in events being scheduled, you have to do it in `model.__init__`. I actually think I like this over the old situation. 
quaquel: > Unfortunately, the performance degradation looks reproducible and significant.
>
> Any idea what causing it?

yes: inccorrect behavior before. What happend was the following

1. grass schedules regrowth events if not fully grown
2. simulator.setup is called, canceling all events
3. Only full grown grass is available to eat and to reschedule itself
4. sheep die very quickly (first 10 turns or so)
5. wolfs die shortly after

Once I had the animation in #2470, the error was obvious. 

Patch:
diff --git a/benchmarks/global_benchmark.py b/benchmarks/global_benchmark.py
index c3f0c071..ec46c019 100644
--- a/benchmarks/global_benchmark.py
+++ b/benchmarks/global_benchmark.py
@@ -33,7 +33,6 @@ def run_model(model_class, seed, parameters):
     if model_class.__name__ in uses_simulator:
         simulator = ABMSimulator()
         model = model_class(simulator=simulator, seed=seed, **parameters)
-        simulator.setup(model)
     else:
         model = model_class(seed=seed, **parameters)
 
diff --git a/mesa/examples/advanced/wolf_sheep/model.py b/mesa/examples/advanced/wolf_sheep/model.py
index 982fcc28..6f8887d0 100644
--- a/mesa/examples/advanced/wolf_sheep/model.py
+++ b/mesa/examples/advanced/wolf_sheep/model.py
@@ -61,12 +61,13 @@ class WolfSheep(Model):
             simulator: ABMSimulator instance for event scheduling
         """
         super().__init__(seed=seed)
+        self.simulator = simulator
+        self.simulator.setup(self)
 
         # Initialize model parameters
         self.height = height
         self.width = width
         self.grass = grass
-        self.simulator = simulator
 
         # Create grid using experimental cell space
         self.grid = OrthogonalVonNeumannGrid(
diff --git a/mesa/experimental/devs/simulator.py b/mesa/experimental/devs/simulator.py
index 8967c19e..57749f03 100644
--- a/mesa/experimental/devs/simulator.py
+++ b/mesa/experimental/devs/simulator.py
@@ -57,8 +57,20 @@ class Simulator:
         Args:
             model (Model): The model to simulate
 
+        Raises:
+            Exception if simulator.time is not equal to simulator.starttime
+            Exception if event list is not empty
+
         """
-        self.event_list.clear()
+        if self.time != self.start_time:
+            raise ValueError(
+                "trying to setup model, but current time is not equal to start_time, Has the simulator been reset or freshly initialized?"
+            )
+        if not self.event_list.is_empty():
+            raise ValueError(
+                "trying to setup model, but events have already been scheduled. Call simulator.setup before any scheduling"
+            )
+
         self.model = model
 
     def reset(self):
@@ -68,7 +80,20 @@ class Simulator:
         self.time = self.start_time
 
     def run_until(self, end_time: int | float) -> None:
-        """Run the simulator until the end time."""
+        """Run the simulator until the end time.
+
+        Args:
+            end_time (int | float): The end time for stopping the simulator
+
+        Raises:
+            Exception if simulator.setup() has not yet been called
+
+        """
+        if self.model is None:
+            raise Exception(
+                "simulator has not been setup, call simulator.setup(model) first"
+            )
+
         while True:
             try:
                 event = self.event_list.pop_event()
@@ -84,6 +109,26 @@ class Simulator:
                 self._schedule_event(event)  # reschedule event
                 break
 
+    def run_next_event(self):
+        """Execute the next event.
+
+        Raises:
+            Exception if simulator.setup() has not yet been called
+
+        """
+        if self.model is None:
+            raise Exception(
+                "simulator has not been setup, call simulator.setup(model) first"
+            )
+
+        try:
+            event = self.event_list.pop_event()
+        except IndexError:  # event list is empty
+            return
+        else:
+            self.time = event.time
+            event.execute()
+
     def run_for(self, time_delta: int | float):
         """Run the simulator for the specified time delta.
 
@@ -92,6 +137,7 @@ class Simulator:
                                      plus the time delta
 
         """
+        # fixme, raise initialization error or something like it if model.setup has not been called
         end_time = self.time + time_delta
         self.run_until(end_time)
 
@@ -228,7 +274,7 @@ class ABMSimulator(Simulator):
 
         """
         super().setup(model)
-        self.schedule_event_now(self.model.step, priority=Priority.HIGH)
+        self.schedule_event_next_tick(self.model.step, priority=Priority.HIGH)
 
     def check_time_unit(self, time) -> bool:
         """Check whether the time is of the correct unit.
@@ -277,7 +323,15 @@ class ABMSimulator(Simulator):
         Args:
             end_time (float| int): The end_time delta. The simulator is until the specified end time
 
+        Raises:
+            Exception if simulator.setup() has not yet been called
+
         """
+        if self.model is None:
+            raise Exception(
+                "simulator has not been setup, call simulator.setup(model) first"
+            )
+
         while True:
             try:
                 event = self.event_list.pop_event()
@@ -285,6 +339,8 @@ class ABMSimulator(Simulator):
                 self.time = end_time
                 break
 
+            # fixme: the alternative would be to wrap model.step with an annotation which
+            #  handles this scheduling.
             if event.time <= end_time:
                 self.time = event.time
                 if event.fn() == self.model.step:
@@ -298,17 +354,6 @@ class ABMSimulator(Simulator):
                 self._schedule_event(event)
                 break
 
-    def run_for(self, time_delta: int):
-        """Run the simulator for the specified time delta.
-
-        Args:
-            time_delta (float| int): The time delta. The simulator is run from the current time to the current time
-                                     plus the time delta
-
-        """
-        end_time = self.time + time_delta - 1
-        self.run_until(end_time)
-
 
 class DEVSimulator(Simulator):
     """A simulator where the unit of time is a float.


Test patch:
diff --git a/tests/test_devs.py b/tests/test_devs.py
index 8f1dd937..883a74d1 100644
--- a/tests/test_devs.py
+++ b/tests/test_devs.py
@@ -1,6 +1,6 @@
 """Tests for experimental Simulator classes."""
 
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, Mock
 
 import pytest
 
@@ -55,6 +55,23 @@ def test_devs_simulator():
     with pytest.raises(ValueError):
         simulator.schedule_event_absolute(fn2, 0.5)
 
+    # step
+    simulator = DEVSimulator()
+    model = MagicMock(spec=Model)
+    simulator.setup(model)
+
+    fn = MagicMock()
+    simulator.schedule_event_absolute(fn, 1.0)
+    simulator.run_next_event()
+    fn.assert_called_once()
+    assert simulator.time == 1.0
+    simulator.run_next_event()
+    assert simulator.time == 1.0
+
+    simulator = DEVSimulator()
+    with pytest.raises(Exception):
+        simulator.run_next_event()
+
     # cancel_event
     simulator = DEVSimulator()
     model = MagicMock(spec=Model)
@@ -70,6 +87,24 @@ def test_devs_simulator():
     assert simulator.model is None
     assert simulator.time == 0.0
 
+    # run without setup
+    simulator = DEVSimulator()
+    with pytest.raises(Exception):
+        simulator.run_until(10)
+
+    # setup with time advanced
+    simulator = DEVSimulator()
+    simulator.time = simulator.start_time + 1
+    model = MagicMock(spec=Model)
+    with pytest.raises(Exception):
+        simulator.setup(model)
+
+    # setup with event scheduled
+    simulator = DEVSimulator()
+    simulator.schedule_event_now(Mock())
+    with pytest.raises(Exception):
+        simulator.setup(model)
+
 
 def test_abm_simulator():
     """Tests abm simulator."""
@@ -86,7 +121,12 @@ def test_abm_simulator():
 
     simulator.run_for(3)
     assert model.step.call_count == 3
-    assert simulator.time == 2
+    assert simulator.time == 3
+
+    # run without setup
+    simulator = ABMSimulator()
+    with pytest.raises(Exception):
+        simulator.run_until(10)
 
 
 def test_simulation_event():
diff --git a/tests/test_examples.py b/tests/test_examples.py
index 436bd805..98d1d580 100644
--- a/tests/test_examples.py
+++ b/tests/test_examples.py
@@ -69,6 +69,5 @@ def test_wolf_sheep():  # noqa: D103
     from mesa.experimental.devs import ABMSimulator
 
     simulator = ABMSimulator()
-    model = WolfSheep(seed=42, simulator=simulator)
-    simulator.setup(model)
+    WolfSheep(seed=42, simulator=simulator)
     simulator.run_for(10)