Repository: projectmesa/mesa
Pull request #1115 (2021-12-22 15:33:15)

 Adding logic to check whether there is agent data
 There was a bug where the model output was only produced
 if there were agent_reporters.

 This is because the output data was created in a list
 comprehension that ran once for every agent in
 agent_reporters.

 The new logic checks if there are agent reporters;
 if not, it just outputs the model_reporters.

 Also adds a simple unit test to make sure that output
 is produced when there aren't agent_reporters.

Fixes #1113 

codecov[bot]: # [Codecov](https://codecov.io/gh/projectmesa/mesa/pull/1115?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) Report
> Merging [#1115](https://codecov.io/gh/projectmesa/mesa/pull/1115?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) (d3afe7f) into [main](https://codecov.io/gh/projectmesa/mesa/commit/52010f5ce1803c8e87dff7cada306cac25859481?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) (52010f5) will **decrease** coverage by `0.15%`.
> The diff coverage is `33.33%`.

> :exclamation: Current head d3afe7f differs from pull request most recent head 321f5eb. Consider uploading reports for the commit 321f5eb to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/projectmesa/mesa/pull/1115/graphs/tree.svg?width=650&height=150&src=pr&token=OUHxz83iDi&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa)](https://codecov.io/gh/projectmesa/mesa/pull/1115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa)

```diff
@@            Coverage Diff             @@
##             main    #1115      +/-   ##
==========================================
- Coverage   89.47%   89.32%   -0.16%     
==========================================
  Files          19       19              
  Lines        1254     1255       +1     
  Branches      256      252       -4     
==========================================
- Hits         1122     1121       -1     
- Misses         97       98       +1     
- Partials       35       36       +1     
```


| [Impacted Files](https://codecov.io/gh/projectmesa/mesa/pull/1115?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa) | Coverage Δ | |
|---|---|---|
| [mesa/batchrunner.py](https://codecov.io/gh/projectmesa/mesa/pull/1115/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa#diff-bWVzYS9iYXRjaHJ1bm5lci5weQ==) | `91.55% <33.33%> (-0.60%)` | :arrow_down: |
| [mesa/space.py](https://codecov.io/gh/projectmesa/mesa/pull/1115/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa#diff-bWVzYS9zcGFjZS5weQ==) | `95.90% <0.00%> (-0.02%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/projectmesa/mesa/pull/1115?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/projectmesa/mesa/pull/1115?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa). Last update [52010f5...321f5eb](https://codecov.io/gh/projectmesa/mesa/pull/1115?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=projectmesa).

rht: Remaining TODOs:
- You need to squash the 8 commits into 1.
- If you want to update your git branch, do it with `git pull --rebase`. This is cleaner and without the messy merge commits.
rht: This is how I usually squash my commits: https://stackoverflow.com/a/5189600.
jdfoote: Hahaha - I worked so hard to try to squash these and didn't even notice that I got it wrong! I'll try again. :)
rht: Looks like you managed to successfully squash.
rht: The coverage report is outdated. I tested your change locally, and that `else` branch is actually covered by the newly added test.
Remaining TODOs:
- Rename `agent_reporters=True` to `enable_agent_reporters=True`
- Use imperative mood for the commit messages. You can reword the commit message by doing the same `git rebase -i HEAD~n`, and then change `pick` into `reword` for the commits you want to reword
 
See https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project
In that article, you can search for the paragraph that contains this sentence for more detail:
>  Write your commit message in the imperative: "Fix bug" and not "Fixed bug" or "Fixes bug."
jdfoote: @rht Thanks so much for patiently helping me to figure this process out.

I changed the commit message for the primary commit. Should I squash again so that all of the changes are in just one commit?
rht: Yeah, you should squash them into 1 commit.

Patch:
diff --git a/examples/bank_reserves/batch_run.py b/examples/bank_reserves/batch_run.py
index d6cbaea8..e028f414 100644
--- a/examples/bank_reserves/batch_run.py
+++ b/examples/bank_reserves/batch_run.py
@@ -186,15 +186,6 @@ br_params = {
     "reserve_percent": 5,
 }
 
-"""br = BatchRunner(
-    BankReservesModel,
-    br_params,
-    iterations=2,
-    max_steps=1000,
-    nr_processes=None,
-    # model_reporters={"Data Collector": lambda m: m.datacollector},
-)"""
-
 if __name__ == "__main__":
     data = batch_run(
         BankReservesModel,
@@ -204,8 +195,20 @@ if __name__ == "__main__":
     )
     br_df = pd.DataFrame(data)
     br_df.to_csv("BankReservesModel_Data.csv")
-    # br.run_all()
+
+    # The commented out code below is the equivalent code as above, but done
+    # via the legacy BatchRunner class. This is a good example to look at if
+    # you want to migrate your code to use `batch_run()` from `BatchRunner`.
     """
+    br = BatchRunner(
+        BankReservesModel,
+        br_params,
+        iterations=2,
+        max_steps=1000,
+        nr_processes=None,
+        # model_reporters={"Data Collector": lambda m: m.datacollector},
+    )
+    br.run_all()
     br_df = br.get_model_vars_dataframe()
     br_step_data = pd.DataFrame()
     for i in range(len(br_df["Data Collector"])):
diff --git a/mesa/batchrunner.py b/mesa/batchrunner.py
index e81e1321..c6293ea5 100644
--- a/mesa/batchrunner.py
+++ b/mesa/batchrunner.py
@@ -172,10 +172,15 @@ def _model_run_func(
     for step in steps:
         model_data, all_agents_data = _collect_data(model, step)
 
-        stepdata = [
-            {**{"Step": step}, **kwargs, **model_data, **agent_data}
-            for agent_data in all_agents_data
-        ]
+        # If there are agent_reporters, then create an entry for each agent
+        if all_agents_data:
+            stepdata = [
+                {**{"Step": step}, **kwargs, **model_data, **agent_data}
+                for agent_data in all_agents_data
+            ]
+        # If there is only model data, then create a single entry for the step
+        else:
+            stepdata = [{**{"Step": step}, **kwargs, **model_data}]
         data.extend(stepdata)
 
     return tuple(kwargs.values()), data


Test patch:
diff --git a/tests/test_batch_run.py b/tests/test_batch_run.py
index d963d3ba..7d24e88f 100644
--- a/tests/test_batch_run.py
+++ b/tests/test_batch_run.py
@@ -47,6 +47,7 @@ class MockModel(Model):
         variable_agent_param=None,
         fixed_model_param=None,
         schedule=None,
+        enable_agent_reporters=True,
         **kwargs
     ):
         super().__init__()
@@ -55,9 +56,13 @@ class MockModel(Model):
         self.variable_agent_param = variable_agent_param
         self.fixed_model_param = fixed_model_param
         self.n_agents = 3
+        if enable_agent_reporters:
+            agent_reporters = {"agent_id": "unique_id", "agent_local": "local"}
+        else:
+            agent_reporters = None
         self.datacollector = DataCollector(
             model_reporters={"reported_model_param": self.get_local_model_param},
-            agent_reporters={"agent_id": "unique_id", "agent_local": "local"},
+            agent_reporters=agent_reporters,
         )
         self.running = True
         self.init_agents()
@@ -121,5 +126,19 @@ def test_batch_run_with_params():
     )
 
 
+def test_batch_run_no_agent_reporters():
+    result = batch_run(MockModel, {"enable_agent_reporters": False})
+    print(result)
+    assert result == [
+        {
+            "RunId": 0,
+            "iteration": 0,
+            "Step": 1000,
+            "enable_agent_reporters": False,
+            "reported_model_param": 42,
+        }
+    ]
+
+
 def test_batch_run_single_core():
     batch_run(MockModel, {}, nr_processes=1, iterations=10)