Repository: projectmesa/mesa
Pull request #2002 (2024-01-25 11:30:36)

benchmarks.yml: Run on addition of label instead of comment
Instead of triggering on a specific comment, which caused issues with running in a different scope ([`issue_comment`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issue_comment) instead of [`pull_request_target`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target)), the workflow can now instead be triggered by adding a label "trigger-benchmarks" to the PR.

The label can be repeatedly added and removed to rerun benchmarks.

![Screenshot_363](https://github.com/projectmesa/mesa/assets/15776622/6009c232-4d10-4791-9a40-71eb82ff6c1f)

github-actions[bot]: Performance benchmarks:

| Model        | Size   | Init time [95% CI]     | Run time [95% CI]      |
|--------------|--------|------------------------|------------------------|
| Schelling    | small  | 🔵 +1.3% [+1.0%, +1.5%] | 🔵 +0.2% [+0.0%, +0.3%] |
| Schelling    | large  | 🔵 +0.7% [+0.4%, +1.0%] | 🔵 -1.2% [-2.4%, +0.0%] |
| WolfSheep    | small  | 🔵 +0.5% [+0.1%, +0.8%] | 🔵 +0.5% [+0.4%, +0.6%] |
| WolfSheep    | large  | 🔵 +0.1% [-1.2%, +1.5%] | 🔵 +0.1% [-1.8%, +2.0%] |
| BoidFlockers | small  | 🔵 +1.1% [+0.8%, +1.4%] | 🔵 +1.9% [+1.3%, +2.5%] |
| BoidFlockers | large  | 🔵 +0.9% [+0.5%, +1.3%] | 🔵 +1.1% [+0.6%, +1.6%] |

Corvince: If this works this is probably a good solution. While your changing the trigger, maybe also now deactivate the trigger on open pull requests? I think a lot of PRs a not related to performance and the performance benchmark is a bit distracting there. Or maybe run on every PR, but don't comment without the label?
rht: What about reacting to the existing benchmark comment with a rocket emoji to retrigger the benchmark? Figuring out a more convenient way to do the retrigger than removing and readding the label.

Patch:
diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml
index 878ba523..e1e146a6 100644
--- a/.github/workflows/benchmarks.yml
+++ b/.github/workflows/benchmarks.yml
@@ -2,17 +2,12 @@ name: Performance benchmarks
 
 on:
   pull_request_target:
-    types: [opened, ready_for_review]
+    types: [opened, ready_for_review, labeled]
     branches:
       - main
     paths:
       - '**.py'
       - '.github/workflows/benchmarks.yml'
-  issue_comment:
-    types: [created]
-    paths:
-      - '**.py'
-      - '.github/workflows/benchmarks.yml'
 
 permissions:
   issues: write
@@ -21,12 +16,12 @@ permissions:
 jobs:
   run-benchmarks:
     if: >
-      github.event_name == 'pull_request_target' ||
-      (github.event_name == 'issue_comment' && contains(github.event.comment.body, '/rerun-benchmarks'))
+      github.event.action == 'labeled' && contains(github.event.pull_request.labels.*.name, 'trigger-benchmarks') ||
+      github.event.action == 'opened' ||
+      github.event.action == 'ready_for_review'
     runs-on: ubuntu-latest
     steps:
-      - name: Checkout PR branch
-        uses: actions/checkout@v4
+      # Python and dependency setup
       - name: Set up Python
         uses: actions/setup-python@v5
         with:
@@ -35,6 +30,7 @@ jobs:
         run: echo "PYTHONPATH=$PYTHONPATH:$(pwd)" >> $GITHUB_ENV
       - name: Install dependencies
         run: pip install numpy pandas tqdm tabulate
+      # Benchmarks on the projectmesa main branch
       - name: Checkout main branch
         uses: actions/checkout@v4
         with:
@@ -43,6 +39,7 @@ jobs:
       - name: Run benchmarks on main branch
         working-directory: benchmarks
         run: python global_benchmark.py
+      # Upload benchmarks, checkout PR branch, download benchmarks
       - name: Upload benchmark results
         uses: actions/upload-artifact@v4
         with:
@@ -57,43 +54,16 @@ jobs:
           fetch-depth: 0
           persist-credentials: false
           clean: false
-      - name: Get PR info for Issue Comment
-        if: github.event_name == 'issue_comment'
-        id: get-pr-info
-        uses: actions/github-script@v7
-        with:
-          script: |
-            const issue_number = context.issue.number;
-            const repository = context.repo.repo;
-            const owner = context.repo.owner;
-            const pr = await github.rest.pulls.list({
-              owner,
-              repo: repository,
-              head: owner + ":" + issue_number
-            });
-            if (pr.data.length === 0) {
-              throw new Error('No PR found for issue number ' + issue_number);
-            }
-            const headRepo = pr.data[0].head.repo.full_name;
-            const headRef = pr.data[0].head.ref;
-            return { headRepo, headRef };
-      - name: Checkout PR branch for Issue Comment
-        if: github.event_name == 'issue_comment'
-        uses: actions/checkout@v4
-        with:
-          repository: ${{ steps.get-pr-info.outputs.headRepo }}
-          ref: ${{ steps.get-pr-info.outputs.headRef }}
-          token: ${{ secrets.GITHUB_TOKEN }}
-          fetch-depth: 0
-          persist-credentials: false
       - name: Download benchmark results
         uses: actions/download-artifact@v4
         with:
           name: timings-main
           path: benchmarks
+      # Run benchmarks on the PR branch
       - name: Run benchmarks on PR branch
         working-directory: benchmarks
         run: python global_benchmark.py
+      # Run compare script and create comment
       - name: Run compare timings and encode output
         working-directory: benchmarks
         run: |
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 2d83ddc8..661543ba 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,6 +1,5 @@
 ci:
     autoupdate_schedule: 'monthly'
-    autofix_prs: false
 
 repos:
 - repo: https://github.com/astral-sh/ruff-pre-commit
@@ -10,6 +9,7 @@ repos:
     # Run the linter.
     - id: ruff
       types_or: [ python, pyi, jupyter ]
+      args: [ --fix ]
     # Run the formatter.
     - id: ruff-format
       types_or: [ python, pyi, jupyter ]
diff --git a/mesa/agent.py b/mesa/agent.py
index fcf643d3..6155284b 100644
--- a/mesa/agent.py
+++ b/mesa/agent.py
@@ -54,6 +54,7 @@ class Agent:
         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/datacollection.py b/mesa/datacollection.py
index 88f1863f..3fcd205c 100644
--- a/mesa/datacollection.py
+++ b/mesa/datacollection.py
@@ -14,8 +14,7 @@ name.
 
 When the collect() method is called, each model-level function is called, with
 the model as the argument, and the results associated with the relevant
-variable. Then the agent-level functions are called on each agent in the model
-scheduler.
+variable. Then the agent-level functions are called on each agent.
 
 Additionally, other objects can write directly to tables by passing in an
 appropriate dictionary object for a table row.
@@ -30,8 +29,7 @@ The DataCollector then stores the data it collects in dictionaries:
 Finally, DataCollector can create a pandas DataFrame from each collection.
 
 The default DataCollector here makes several assumptions:
-    * The model has a schedule object called 'schedule'
-    * The schedule has an agent list called agents
+    * The model has an agent list called agents
     * For collecting agent-level variables, agents must have a unique_id
 """
 import contextlib
@@ -67,7 +65,7 @@ class DataCollector:
 
         Model reporters can take four types of arguments:
         1. Lambda function:
-           {"agent_count": lambda m: m.schedule.get_agent_count()}
+           {"agent_count": lambda m: len(m.agents)}
         2. Method of a class/instance:
            {"agent_count": self.get_agent_count} # self here is a class instance
            {"agent_count": Model.get_agent_count} # Model here is a class
@@ -180,11 +178,14 @@ class DataCollector:
         rep_funcs = self.agent_reporters.values()
 
         def get_reports(agent):
-            _prefix = (agent.model.schedule.steps, agent.unique_id)
+            _prefix = (agent.model._steps, agent.unique_id)
             reports = tuple(rep(agent) for rep in rep_funcs)
             return _prefix + reports
 
-        agent_records = map(get_reports, model.schedule.agents)
+        agent_records = map(
+            get_reports,
+            model.schedule.agents if hasattr(model, "schedule") else model.agents,
+        )
         return agent_records
 
     def collect(self, model):
@@ -207,7 +208,7 @@ class DataCollector:
 
         if self.agent_reporters:
             agent_records = self._record_agents(model)
-            self._agent_records[model.schedule.steps] = list(agent_records)
+            self._agent_records[model._steps] = list(agent_records)
 
     def add_table_row(self, table_name, row, ignore_missing=False):
         """Add a row dictionary to a specific table.
diff --git a/mesa/experimental/jupyter_viz.py b/mesa/experimental/jupyter_viz.py
index 7a586239..eb20ca5f 100644
--- a/mesa/experimental/jupyter_viz.py
+++ b/mesa/experimental/jupyter_viz.py
@@ -181,7 +181,7 @@ def ModelController(model, play_interval, current_step, reset_counter):
     def do_step():
         model.step()
         previous_step.value = current_step.value
-        current_step.value += 1
+        current_step.value = model._steps
 
     def do_play():
         model.running = True
diff --git a/mesa/model.py b/mesa/model.py
index 1bd9b3a7..b2b12340 100644
--- a/mesa/model.py
+++ b/mesa/model.py
@@ -13,11 +13,13 @@ import warnings
 from collections import defaultdict
 
 # mypy
-from typing import Any
+from typing import Any, Union
 
 from mesa.agent import Agent, AgentSet
 from mesa.datacollection import DataCollector
 
+TimeT = Union[float, int]
+
 
 class Model:
     """Base class for models in the Mesa ABM library.
@@ -68,6 +70,9 @@ class Model:
         self.current_id = 0
         self.agents_: defaultdict[type, dict] = defaultdict(dict)
 
+        self._steps: int = 0
+        self._time: TimeT = 0  # the model's clock
+
         # Warning flags for current experimental features. These make sure a warning is only printed once per model.
         self.agentset_experimental_warning_given = False
 
@@ -112,6 +117,11 @@ class Model:
     def step(self) -> None:
         """A single step. Fill in here."""
 
+    def _advance_time(self, deltat: TimeT = 1):
+        """Increment the model's steps counter and clock."""
+        self._steps += 1
+        self._time += deltat
+
     def next_id(self) -> int:
         """Return the next unique ID for agents, increment current_id"""
         self.current_id += 1
diff --git a/mesa/time.py b/mesa/time.py
index 87a68949..50d564be 100644
--- a/mesa/time.py
+++ b/mesa/time.py
@@ -73,6 +73,8 @@ class BaseScheduler:
         self.model = model
         self.steps = 0
         self.time: TimeT = 0
+        self._original_step = self.step
+        self.step = self._wrapped_step
 
         if agents is None:
             agents = []
@@ -115,6 +117,11 @@ class BaseScheduler:
         self.steps += 1
         self.time += 1
 
+    def _wrapped_step(self):
+        """Wrapper for the step method to include time and step updating."""
+        self._original_step()
+        self.model._advance_time()
+
     def get_agent_count(self) -> int:
         """Returns the current number of agents in the queue."""
         return len(self._agents)
@@ -143,8 +150,8 @@ class BaseScheduler:
 
     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):


Test patch:
diff --git a/tests/test_agent.py b/tests/test_agent.py
index 5861038d..60a0eec1 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 3cfc2f35..53967d1a 100644
--- a/tests/test_time.py
+++ b/tests/test_time.py
@@ -224,6 +224,28 @@ class TestRandomActivation(TestCase):
         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):
     """