Refactor examples with common interface, plotting and benchmarking #188
Conversation
- Implemented a new backend using Mesa with sequential updates in `examples/sugarscape_ig/backend_mesa`. - Created agent and model classes for the Sugarscape simulation, including movement and sugar management. - Added a CLI interface using Typer for running simulations and saving results. - Introduced utility classes for handling simulation results from both Mesa and Mesa-Frames backends. - Added a new backend using Mesa-Frames with parallel updates in `examples/sugarscape_ig/backend_frames`. - Implemented model-level reporters for Gini coefficient and correlations between agent traits. - Included CSV output and plotting capabilities for simulation metrics.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #188 +/- ##
=======================================
Coverage 89.23% 89.23%
=======================================
Files 14 14
Lines 2007 2007
=======================================
Hits 1791 1791
Misses 216 216 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| docs = [ | ||
| { name = "autodocsumm" }, | ||
| { name = "beartype" }, | ||
| { name = "jupytext" }, | ||
| { name = "mesa" }, |
There was a problem hiding this comment.
The docs dependency group in uv.lock no longer includes perfplot, even though pyproject.toml still declares perfplot>=0.10.2 as part of the docs group (line 81). With this mismatch, running uv sync --locked --group docs/dev will fail or produce an environment that violates the declared dependencies because the lock is missing a required package. Regenerate the lock so the docs group matches the project manifest.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request introduces a comprehensive benchmarking and examples framework for Mesa and Mesa-Frames. It adds a Typer-based CLI for running performance benchmarks across two models (Boltzmann Wealth Exchange and Sugarscape IG) with dual backend implementations, shared plotting utilities, and extensive documentation covering usage, metrics, and extension guidance. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as benchmarks/cli.py
participant Models as Model Factory
participant Backend as Backend<br/>(mesa/frames)
participant DataCollector as DataCollector
participant Results as File I/O
participant Plotting as plotting.py
User->>CLI: run(models, agents, steps, repeats, seed, save, plot)
CLI->>CLI: Parse & validate inputs (agents, models)
loop For each model
CLI->>Models: Instantiate model config
loop For each backend in model
loop For each repeat
CLI->>Backend: Execute runner(agents, steps, seed)
activate Backend
Backend->>DataCollector: Collect metrics during step()
Backend-->>CLI: Return runtime
deactivate Backend
CLI->>CLI: Record runtime
end
end
end
CLI->>CLI: Aggregate results into DataFrame
alt save=true
CLI->>Results: Write CSV to timestamped directory
end
alt plot=true
CLI->>Plotting: plot_performance(df, model_name, output_dir, timestamp)
activate Plotting
Plotting->>Plotting: Melt & aggregate by backend
Plotting->>Plotting: Generate light/dark theme plots
Plotting->>Results: Save PNG/SVG
deactivate Plotting
end
CLI->>User: Report completion with file paths
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Key areas requiring focused attention:
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (12)
examples/__init__.py (1)
1-6: Clean and simple package initializer.The module docstring and
__all__list are properly structured and follow Python conventions. The subpackage names use the correct snake_case naming style.Note: The
__all__list declares these as public API surface, but without explicit imports, users will need to access them via the full path (e.g.,import examples.boltzmann_wealth) rather thanfrom examples import boltzmann_wealth. If direct importing from the examples package is desired, consider adding explicit imports:from . import boltzmann_wealth, sugarscape_igThis is optional based on your intended API—just ensure the usage pattern is intentional and documented.
examples/sugarscape_ig/backend_frames/agents.py (2)
298-298: Redundant.with_columns(pl.col("radius"))does nothing.This line selects
radiuswithout any transformation, effectively a no-op.- .with_columns(pl.col("radius"))
311-311: Minor: Non-breaking hyphen in comment.The comment contains a non-breaking hyphen (
‑) instead of a regular hyphen-minus (-). This is flagged by Ruff (RUF003) but is purely cosmetic.- # Precompute per‑agent candidate rank once so conflict resolution can + # Precompute per-agent candidate rank once so conflict resolution canexamples/boltzmann_wealth/backend_frames.py (1)
5-13: Non-standard import order.Standard library imports (
os,time) are interleaved with third-party imports. PEP 8 recommends grouping imports: stdlib first, then third-party.from datetime import datetime, timezone +import os from pathlib import Path +from time import perf_counter from typing import Annotated import numpy as np -import os import polars as pl import typer -from time import perf_counterexamples/plotting.py (3)
71-75: Consider logging the exception instead of silently ignoring it.The
try-except-passpattern suppresses all errors during SVG export. While SVG is optional, logging the exception would aid debugging when exports fail unexpectedly.- try: - fig.savefig(output_dir / f"{stem}_{theme}.svg", bbox_inches="tight") - except Exception: - pass # SVG is a nice-to-have + try: + fig.savefig(output_dir / f"{stem}_{theme}.svg", bbox_inches="tight") + except Exception as exc: # noqa: BLE001 + import logging + logging.debug("SVG export failed for %s: %s", stem, exc)
109-114: BareExceptioncatch may mask unexpected errors.The intent is to handle missing/malformed step data, but catching
Exceptioncould hide unrelated bugs. Consider catching more specific exceptions or at minimum adding a# noqa: BLE001comment to acknowledge this is intentional.if steps is None: try: steps = int(metrics.select(pl.col("step").max()).item()) + 1 - except Exception: + except (TypeError, ValueError, pl.exceptions.ComputeError): steps = None
286-290: Sort__all__for consistency with isort conventions.Static analysis flagged this as unsorted. Sorting maintains consistency with standard Python tooling.
__all__ = [ + "plot_agent_metrics", "plot_model_metrics", - "plot_agent_metrics", "plot_performance", ]examples/sugarscape_ig/backend_mesa/model.py (2)
57-72: Docstring mismatch: function signature differs from similar functions.The docstring for
ginidoesn't follow NumPy style as required by the coding guidelines. Consider adding Parameters/Returns sections for consistency with public APIs.def gini(values: Iterable[float]) -> float: + """Compute the Gini coefficient from an iterable of wealth values. + + Parameters + ---------- + values : Iterable[float] + Iterable of wealth/sugar values. + + Returns + ------- + float + Gini coefficient in [0, 1], or 0.0 for zero-total/constant values, + or NaN for empty input. + """ array = np.fromiter(values, dtype=float)
132-134: Unused lambda parameterm– consider using_convention.The
seedreporter lambda capturesseedfrom the enclosing scope but declares an unused parameterm. Using_makes the intent explicit.- "seed": lambda m: seed, + "seed": lambda _: seed,examples/boltzmann_wealth/backend_mesa.py (2)
24-40: Consider extractingginito a shared module to reduce duplication.This
ginifunction is identical to the one inexamples/sugarscape_ig/backend_mesa/model.py(lines 57-72). Extracting it toexamples/utils.pywould improve maintainability.You could add to
examples/utils.py:def gini(values: Iterable[float]) -> float: """Compute the Gini coefficient from an iterable of wealth values.""" array = np.fromiter(values, dtype=float) if array.size == 0: return float("nan") if np.allclose(array, 0.0): return 0.0 if np.allclose(array, array[0]): return 0.0 sorted_vals = np.sort(array) n = sorted_vals.size cumulative = np.cumsum(sorted_vals) total = cumulative[-1] if total == 0: return 0.0 index = np.arange(1, n + 1, dtype=float) return float((2.0 * np.dot(index, sorted_vals) / (n * total)) - (n + 1) / n)Then import from both backends.
74-77: Unused lambda parameterm– use_convention.Same issue as in the Sugarscape model.
model_reporters={ "gini": lambda m: gini(a.wealth for a in m.agent_list), - "seed": lambda m: seed, + "seed": lambda _: seed, }benchmarks/cli.py (1)
97-100: Range generation logic could be simplified.The current approach generates
range(start, stop + step, step)then pops if the last value exceedsstop. This is correct but less idiomatic than usingrange(start, stop + 1, step)directly for inclusive ranges.- counts = list(range(start, stop + step, step)) - if counts[-1] > stop: - counts.pop() - return counts + return list(range(start, stop + 1, step))This produces the same result for an inclusive
[start, stop]range with the given step.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.gitignore(1 hunks)benchmarks/README.md(1 hunks)benchmarks/cli.py(1 hunks)examples/README.md(1 hunks)examples/__init__.py(1 hunks)examples/boltzmann_wealth/README.md(1 hunks)examples/boltzmann_wealth/backend_frames.py(1 hunks)examples/boltzmann_wealth/backend_mesa.py(1 hunks)examples/plotting.py(1 hunks)examples/sugarscape_ig/README.md(1 hunks)examples/sugarscape_ig/backend_frames/__init__.py(1 hunks)examples/sugarscape_ig/backend_frames/agents.py(1 hunks)examples/sugarscape_ig/backend_frames/model.py(1 hunks)examples/sugarscape_ig/backend_mesa/__init__.py(1 hunks)examples/sugarscape_ig/backend_mesa/agents.py(1 hunks)examples/sugarscape_ig/backend_mesa/model.py(1 hunks)examples/utils.py(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python 3.11 or later with 4-space indentation
Include type hints for all public APIs
Use NumPy-style docstrings (validated by Ruff/pydoclint)
UseCamelCasefor class names
Usesnake_casefor function and attribute names
Avoid usingTYPE_CHECKINGguards for type annotations because the project usesbeartypefor runtime type checking, which requires the actual type objects to be available at runtime
Treat underscored attributes as internal/private and not part of the public API
Files:
examples/utils.pyexamples/__init__.pyexamples/sugarscape_ig/backend_mesa/agents.pyexamples/plotting.pyexamples/sugarscape_ig/backend_mesa/__init__.pyexamples/boltzmann_wealth/backend_frames.pyexamples/sugarscape_ig/backend_mesa/model.pyexamples/sugarscape_ig/backend_frames/agents.pyexamples/sugarscape_ig/backend_frames/model.pyexamples/boltzmann_wealth/backend_mesa.pyexamples/sugarscape_ig/backend_frames/__init__.pybenchmarks/cli.py
🧠 Learnings (3)
📚 Learning: 2025-12-08T18:41:11.772Z
Learnt from: CR
Repo: projectmesa/mesa-frames PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T18:41:11.772Z
Learning: Use `examples/` directory for reproducible demo models and performance scripts
Applied to files:
examples/README.mdexamples/__init__.py
📚 Learning: 2025-12-08T18:41:11.772Z
Learnt from: CR
Repo: projectmesa/mesa-frames PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T18:41:11.772Z
Learning: Use `mesa_frames/` as the source package directory, with `abstract/` and `concrete/` subdirectories for core APIs and implementations, and key modules: `agents.py`, `agentset.py`, `space.py`, `datacollector.py`, `types_.py`
Applied to files:
examples/README.mdbenchmarks/README.mdexamples/utils.pyexamples/sugarscape_ig/README.mdexamples/sugarscape_ig/backend_mesa/agents.pyexamples/sugarscape_ig/backend_mesa/__init__.pyexamples/boltzmann_wealth/backend_frames.pyexamples/sugarscape_ig/backend_mesa/model.pyexamples/sugarscape_ig/backend_frames/agents.pyexamples/sugarscape_ig/backend_frames/model.pyexamples/sugarscape_ig/backend_frames/__init__.py
📚 Learning: 2025-04-29T09:25:34.183Z
Learnt from: adamamer20
Repo: projectmesa/mesa-frames PR: 143
File: mesa_frames/abstract/space.py:50-63
Timestamp: 2025-04-29T09:25:34.183Z
Learning: The project mesa-frames has been upgraded to Python 3.11, which provides native support for `Self` type in the standard typing module, eliminating the need for imports from typing_extensions.
Applied to files:
examples/utils.pyexamples/sugarscape_ig/backend_frames/__init__.py
🧬 Code graph analysis (7)
examples/utils.py (1)
mesa_frames/concrete/datacollector.py (1)
DataCollector(69-640)
examples/sugarscape_ig/backend_mesa/agents.py (3)
examples/sugarscape_ig/backend_frames/model.py (2)
Sugarscape(174-386)step(333-350)examples/sugarscape_ig/backend_mesa/model.py (2)
Sugarscape(75-187)step(172-181)examples/sugarscape_ig/backend_frames/agents.py (3)
step(55-71)move(73-80)move(131-185)
examples/boltzmann_wealth/backend_frames.py (3)
mesa_frames/concrete/datacollector.py (1)
DataCollector(69-640)examples/utils.py (1)
FramesSimulationResult(7-14)examples/boltzmann_wealth/backend_mesa.py (1)
gini(24-40)
examples/sugarscape_ig/backend_frames/agents.py (3)
mesa_frames/concrete/agentset.py (1)
AgentSet(76-686)mesa_frames/concrete/model.py (1)
Model(54-223)examples/sugarscape_ig/backend_mesa/agents.py (1)
move(75-78)
examples/sugarscape_ig/backend_frames/model.py (3)
examples/utils.py (1)
FramesSimulationResult(7-14)examples/plotting.py (1)
plot_model_metrics(81-171)mesa_frames/abstract/datacollector.py (2)
collect(118-128)flush(176-196)
examples/boltzmann_wealth/backend_mesa.py (4)
examples/utils.py (1)
MesaSimulationResult(18-25)examples/plotting.py (1)
plot_model_metrics(81-171)examples/boltzmann_wealth/backend_frames.py (3)
gini(24-42)step(52-76)step(106-108)examples/sugarscape_ig/backend_frames/model.py (2)
gini(30-71)step(333-350)
benchmarks/cli.py (3)
examples/plotting.py (1)
plot_performance(234-283)examples/boltzmann_wealth/backend_frames.py (5)
simulate(115-124)step(52-76)step(106-108)run(110-112)run(131-186)examples/boltzmann_wealth/backend_mesa.py (5)
simulate(92-97)step(50-57)step(81-85)run(87-89)run(104-177)
🪛 LanguageTool
examples/boltzmann_wealth/README.md
[grammar] ~57-~57: Use a hyphen to join words.
Context: ...rics - --results-dir Override the auto timestamped directory under results/ ...
(QB_NEW_EN_HYPHEN)
examples/sugarscape_ig/README.md
[grammar] ~50-~50: Use a hyphen to join words.
Context: ...alive` typically decreases until a quasi steady state (metabolism vs regrowth) or...
(QB_NEW_EN_HYPHEN)
🪛 Ruff (0.14.7)
examples/sugarscape_ig/backend_mesa/agents.py
21-21: Undefined name Sugarscape
(F821)
examples/plotting.py
73-74: try-except-pass detected, consider logging the exception
(S110)
73-73: Do not catch blind exception: Exception
(BLE001)
113-113: Do not catch blind exception: Exception
(BLE001)
273-273: Comment contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF003)
286-290: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
examples/sugarscape_ig/backend_mesa/model.py
134-134: Unused lambda argument: m
(ARG005)
260-260: Local variable seed_val is assigned to but never used
Remove assignment to unused variable seed_val
(F841)
examples/sugarscape_ig/backend_frames/agents.py
50-52: Avoid specifying long messages outside the exception class
(TRY003)
311-311: Comment contains ambiguous ‑ (NON-BREAKING HYPHEN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
examples/sugarscape_ig/backend_frames/model.py
227-229: Avoid specifying long messages outside the exception class
(TRY003)
examples/boltzmann_wealth/backend_mesa.py
76-76: Unused lambda argument: m
(ARG005)
138-138: Comment contains ambiguous ‑ (NON-BREAKING HYPHEN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
143-143: Comment contains ambiguous ‑ (NON-BREAKING HYPHEN). Did you mean - (HYPHEN-MINUS)?
(RUF003)
benchmarks/cli.py
86-86: Avoid specifying long messages outside the exception class
(TRY003)
90-90: Avoid specifying long messages outside the exception class
(TRY003)
92-92: Avoid specifying long messages outside the exception class
(TRY003)
94-94: Avoid specifying long messages outside the exception class
(TRY003)
96-96: Avoid specifying long messages outside the exception class
(TRY003)
104-104: Avoid specifying long messages outside the exception class
(TRY003)
106-106: Avoid specifying long messages outside the exception class
(TRY003)
126-126: Avoid specifying long messages outside the exception class
(TRY003)
129-129: Avoid specifying long messages outside the exception class
(TRY003)
197-197: Do not perform function call in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (43)
.gitignore (1)
158-162: Ignore patterns updated appropriately for nested results/plots output.The glob patterns now correctly accommodate timestamped subdirectories (
benchmarks/results/TIMESTAMP/...andexamples/*/results/TIMESTAMP/...) and plot outputs, aligning with the new CLI-driven output structure described in the READMEs.examples/README.md (1)
1-106: Well-structured and comprehensive examples documentation.The README clearly explains the dual-backend approach, CLI interface, data formats, and extension patterns. The quick start, tips, and programmatic use examples are practical and well-documented.
pyproject.toml (1)
66-85: Typer dependency added appropriately to docs group.The placement in the docs dependency group aligns with using Typer for CLI tooling in examples and benchmarks. The version constraint
>=0.9.0is reasonable and not overly restrictive.Please confirm that the examples and benchmarks code actually imports and uses Typer as documented in the READMEs (e.g.,
benchmarks/cli.py,examples/boltzmann_wealth/backend_frames.py), since those files are not visible in this review context.examples/sugarscape_ig/backend_mesa/__init__.py (1)
1-1: Appropriate minimal package initializer.The module docstring correctly identifies the package purpose. No additional code or exports are needed at this stage.
benchmarks/README.md (1)
1-88: Comprehensive and well-structured benchmarking documentation.The README clearly explains purpose, CLI usage, output formats, and extension patterns. The table of CLI options and CSV schema documentation are particularly helpful for reproducibility and post-processing.
examples/sugarscape_ig/backend_frames/__init__.py (1)
1-1: Appropriate minimal package initializer.The module docstring correctly identifies the Frames backend package. No additional code or exports are needed at this stage.
examples/sugarscape_ig/backend_mesa/agents.py (3)
34-47: LGTM!The
_visible_cellsmethod correctly implements cardinal direction visibility with proper boundary checks.
49-73: LGTM!The
_choose_best_cellmethod correctly implements the Sugarscape movement rule with proper tie-breaking by sugar amount, Manhattan distance, and lexicographic coordinates.
75-78: LGTM!The
movemethod is clean and delegates appropriately to the helper methods.examples/utils.py (1)
6-25: LGTM!The dataclasses provide clean containers for simulation results with appropriate type hints. The docstrings could be updated to reference
datacollector.datainstead ofmetrics/agent_metricsfor clarity, but this is a minor documentation nit.examples/sugarscape_ig/backend_frames/agents.py (7)
30-53: LGTM!Good defensive validation of required trait columns with a clear error message. Using
clone()on the input DataFrame prevents unintended side effects.
55-71: LGTM!The step sequence (shuffle → move → eat → remove starved) correctly implements the Sugarscape update order as documented.
82-116: LGTM!The
eatmethod correctly implements vectorised sugar harvesting with proper updates to both agent sugar stocks and cell sugar values.
118-127: LGTM!Clean vectorised removal of starved agents.
131-185: LGTM!The
movemethod is well-structured with clear early exits and proper delegation to helper methods. The inline schema comments enhance readability.
187-248: LGTM!The
_build_neighborhood_framemethod correctly assembles the neighborhood data with proper joins and null handling.
348-620: LGTM!The conflict resolution algorithm is well-implemented with proper fallback handling. The iterative lottery approach with rank promotion ensures all agents are eventually assigned, either to a preferred cell or their origin. The inline schema comments significantly aid comprehension.
examples/sugarscape_ig/backend_frames/model.py (4)
30-71: LGTM with minor redundancy.The function is robust with multiple safety checks. Lines 60-65 contain redundant size checks (already verified at line 58), but this defensive approach is acceptable for example code.
74-140: LGTM!Both correlation reporters are correctly implemented with appropriate null checks. The similar structure between them is acceptable for readability in example code.
143-171: LGTM!The
_safe_corrhelper correctly handles edge cases (insufficient data, constant values) that would make Pearson correlation undefined.
174-386: LGTM!The
Sugarscapeclass is well-structured with:
- Proper validation of agent count vs grid capacity
- Clean separation of grid generation, agent creation, and data collection
- Correct instant-growback sugar regrowth logic
- Appropriate termination handling when agents die out
examples/boltzmann_wealth/backend_frames.py (4)
24-42: LGTM!The
ginifunction correctly handles edge cases and matches the pattern from the Mesa backend implementation.
45-76: LGTM!The
MoneyAgentsclass correctly implements the Boltzmann wealth exchange using vectorised Polars operations. The sampling uses the model RNG for reproducibility.
79-112: LGTM!The
MoneyModelclass correctly integrates the agent set with data collection, with appropriate storage backend selection based onresults_dir.
115-124: LGTM!Clean simulation function that correctly wires the model and returns the result container.
examples/plotting.py (5)
17-44: LGTM! Theme configuration is well-structured.The light/dark theme definitions with consistent styling (disabled top/right spines, appropriate colors) provide a good foundation for the plotting utilities.
47-56: LGTM! Seed shortening helper is correctly implemented.The regex-based approach handles edge cases (None, no match) and truncates long seeds appropriately.
81-171: LGTM!plot_model_metricsimplementation is comprehensive.The function correctly handles:
- Empty dataframes (early return)
- Missing step column (auto-generation)
- Single vs multiple metrics (legend removal, y-axis labeling)
- Title composition with optional N/T metadata
- Multi-theme output
177-228: LGTM!plot_agent_metricshandles id variable detection well.The fallback to the first column when preferred id vars aren't present is a sensible default.
234-283: LGTM!plot_performanceimplementation is solid.The function correctly uses seaborn's
estimator="mean"witherrorbar="sd"for aggregating repeated measurements, and applies appropriate legend styling per theme.examples/sugarscape_ig/backend_mesa/model.py (5)
31-42: LGTM!_safe_corrhandles edge cases correctly.The function properly handles degenerate inputs (too few elements, constant arrays) by returning NaN, matching the documented Frames helper behavior.
45-54: Forward referenceSugarscaperequires string annotation or import reordering.
Sugarscapeis used as a type hint before the class is defined. Whilefrom __future__ import annotations(line 9) enables postponed evaluation making this work, ensure this import remains at the top.
75-144: LGTM! Sugarscape model initialization is well-structured.The model correctly:
- Generates a seed if not provided
- Initializes sugar fields with NumPy RNG
- Places agents on empty cells
- Sets up comprehensive DataCollector reporters
148-181: LGTM! Step scheduling logic matches documented order.The move → eat/starve → regrow → collect sequence matches the docstring and tutorial schedule. The survivor filtering in
_harvest_and_surviveis correctly implemented.
209-311: LGTM! CLI implementation is comprehensive and consistent.The Typer CLI follows the same patterns as the Boltzmann example:
- Timestamped output directories
- Optional CSV persistence and plotting
- Per-metric plot generation
- Clear console output
examples/boltzmann_wealth/backend_mesa.py (3)
43-57: LGTM! MoneyAgent implementation is correct.The agent correctly:
- Initializes with 1 unit of wealth
- Guards against transferring with zero/negative wealth
- Randomly selects a recipient from the agent list
60-90: LGTM! MoneyModel implementation is clean.The model correctly initializes agents, sets up the DataCollector, and implements the shuffled step execution.
103-177: LGTM! CLI implementation follows consistent patterns.The run command properly:
- Creates timestamped output directories
- Measures and reports runtime
- Extracts and displays metrics
- Saves CSV and generates plots conditionally
benchmarks/cli.py (5)
27-34: LGTM! Runner protocol is well-defined.The
RunnerPprotocol cleanly abstracts the runner interface for both Mesa and Frames backends.
43-78: LGTM! Model configuration with lambda wrappers is appropriate.The lambda wrappers for Sugarscape runners correctly adapt the width/height requirements to match the simpler
RunnerPsignature. The grid sizing formula (max(20, ceil(sqrt(agents)) * 2)) ensures adequate space.
200-205: LGTM! Runtime type-checking warning is helpful.Good practice to warn users that
beartyperuntime checking can significantly impact benchmark results.
213-244: LGTM! Benchmark loop implementation is correct.The nested loop correctly iterates over models, agent counts, repeats, and backends, recording all relevant metadata for each run.
246-262: LGTM! Output handling is well-organized.The unified timestamp directory structure with per-model CSVs and a plots subfolder is clean and maintainable.
| def _parse_agents(value: str) -> list[int]: | ||
| value = value.strip() | ||
| if ":" in value: | ||
| parts = value.split(":") | ||
| if len(parts) != 3: | ||
| raise typer.BadParameter("Ranges must use start:stop:step format") | ||
| try: | ||
| start, stop, step = (int(part) for part in parts) | ||
| except ValueError as exc: | ||
| raise typer.BadParameter("Range values must be integers") from exc | ||
| if step <= 0: | ||
| raise typer.BadParameter("Step must be positive") | ||
| if start < 0 or stop <= 0: | ||
| raise typer.BadParameter("Range endpoints must be positive") | ||
| if start > stop: | ||
| raise typer.BadParameter("Range start must be <= stop") | ||
| counts = list(range(start, stop + step, step)) | ||
| if counts[-1] > stop: | ||
| counts.pop() | ||
| return counts | ||
| try: | ||
| agents = int(value) | ||
| except ValueError as exc: # pragma: no cover - defensive | ||
| raise typer.BadParameter("Agent count must be an integer") from exc | ||
| if agents <= 0: | ||
| raise typer.BadParameter("Agent count must be positive") | ||
| return [agents] |
There was a problem hiding this comment.
Off-by-one edge case in range parsing.
When start == 0 is provided, the validation if start < 0 or stop <= 0 passes, but the subsequent logic may produce unexpected results. Consider whether start == 0 should be allowed (0 agents makes no sense).
- if start < 0 or stop <= 0:
- raise typer.BadParameter("Range endpoints must be positive")
+ if start <= 0 or stop <= 0:
+ raise typer.BadParameter("Range endpoints must be positive (> 0)")🧰 Tools
🪛 Ruff (0.14.7)
86-86: Avoid specifying long messages outside the exception class
(TRY003)
90-90: Avoid specifying long messages outside the exception class
(TRY003)
92-92: Avoid specifying long messages outside the exception class
(TRY003)
94-94: Avoid specifying long messages outside the exception class
(TRY003)
96-96: Avoid specifying long messages outside the exception class
(TRY003)
104-104: Avoid specifying long messages outside the exception class
(TRY003)
106-106: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In benchmarks/cli.py around lines 81 to 107, the range parsing allows start == 0
which is invalid for agent counts; change the validation to reject non-positive
endpoints by replacing the check `if start < 0 or stop <= 0` with `if start <= 0
or stop <= 0` and update the BadParameter message if needed to say "Range
endpoints must be positive (>=1)"; keep the rest of the parsing logic (including
step validation and range construction) intact so ranges start at 1 or higher.
| results_dir: Annotated[ | ||
| Path, | ||
| typer.Option( | ||
| help=( | ||
| "Base directory for benchmark outputs. A timestamped subdirectory " | ||
| "(e.g. results/20250101_120000) is created with CSV files at the root " | ||
| "and a 'plots/' subfolder for images." | ||
| ), | ||
| ), | ||
| ] = Path(__file__).resolve().parent / "results", |
There was a problem hiding this comment.
Avoid function call in default argument.
Path(__file__).resolve().parent is evaluated once at module load time. If the module is imported from different working directories, this could produce unexpected paths. Move the resolution inside the function body.
results_dir: Annotated[
- Path,
+ Path | None,
typer.Option(
help=(
"Base directory for benchmark outputs. A timestamped subdirectory "
"(e.g. results/20250101_120000) is created with CSV files at the root "
"and a 'plots/' subfolder for images."
),
),
- ] = Path(__file__).resolve().parent / "results",
+ ] = None,
) -> None:
"""Run performance benchmarks for the models models."""
+ if results_dir is None:
+ results_dir = Path(__file__).resolve().parent / "results"
runtime_typechecking = os.environ.get("MESA_FRAMES_RUNTIME_TYPECHECKING", "")🧰 Tools
🪛 Ruff (0.14.7)
197-197: Do not perform function call in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
In benchmarks/cli.py around lines 188 to 197, the default argument
Path(__file__).resolve().parent / "results" is computed at module import time;
change the parameter to accept an optional Path (e.g., results_dir:
Optional[Path] = None) and move the Path(__file__).resolve().parent / "results"
computation into the function body so it runs at call time (ensure you import
Optional from typing). If results_dir is None inside the function, set
results_dir = Path(__file__).resolve().parent / "results" (or
Path(__file__).parent / "results" if you prefer to avoid resolve) before using
it.
| ), | ||
| ] = Path(__file__).resolve().parent / "results", | ||
| ) -> None: | ||
| """Run performance benchmarks for the models models.""" |
There was a problem hiding this comment.
Typo in docstring: "models models" should be "selected models".
- """Run performance benchmarks for the models models."""
+ """Run performance benchmarks for the selected models."""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| """Run performance benchmarks for the models models.""" | |
| """Run performance benchmarks for the selected models.""" |
🤖 Prompt for AI Agents
In benchmarks/cli.py around line 199, the docstring contains a typo "models
models"; update it to read "selected models" (e.g., change the docstring to
"""Run performance benchmarks for the selected models."""), preserving
surrounding formatting and punctuation.
| if save_results: | ||
| result.datacollector.flush() | ||
|
|
||
| if plot: | ||
| stem = f"gini_{timestamp}" | ||
| # write plots into the results directory so outputs are colocated | ||
| plot_model_metrics( | ||
| model_metrics, | ||
| results_dir, | ||
| stem, | ||
| title="Boltzmann wealth — Gini", | ||
| subtitle=f"mesa-frames backend; seed={result.datacollector.seed}", | ||
| agents=agents, | ||
| steps=steps, | ||
| ) | ||
| typer.echo(f"Saved plots under {results_dir}") | ||
|
|
||
| # Inform user where CSVs were saved | ||
| typer.echo(f"Saved CSV results under {results_dir}") |
There was a problem hiding this comment.
Misleading "Saved CSV results" message when save_results=False.
Similar to the Sugarscape CLI, line 186 always prints the CSV save message regardless of whether save_results is True.
if save_results:
result.datacollector.flush()
+ typer.echo(f"Saved CSV results under {results_dir}")
if plot:
stem = f"gini_{timestamp}"
# write plots into the results directory so outputs are colocated
plot_model_metrics(
model_metrics,
results_dir,
stem,
title="Boltzmann wealth — Gini",
subtitle=f"mesa-frames backend; seed={result.datacollector.seed}",
agents=agents,
steps=steps,
)
typer.echo(f"Saved plots under {results_dir}")
-
- # Inform user where CSVs were saved
- typer.echo(f"Saved CSV results under {results_dir}")🤖 Prompt for AI Agents
In examples/boltzmann_wealth/backend_frames.py around lines 168 to 186, the
final typer.echo always prints "Saved CSV results under {results_dir}" even when
save_results is False; update the logic so the CSV-save message is only emitted
when save_results is True (either move the typer.echo into the existing
save_results block or wrap it in an if save_results: guard) to avoid misleading
output.
| import matplotlib.pyplot as plt | ||
| import mesa | ||
| from mesa.datacollection import DataCollector | ||
| import numpy as np | ||
| import polars as pl | ||
| import seaborn as sns |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused imports matplotlib.pyplot and seaborn.
These modules are imported but never used in this file – plotting is delegated to plot_model_metrics.
-import matplotlib.pyplot as plt
import mesa
from mesa.datacollection import DataCollector
import numpy as np
import polars as pl
-import seaborn as sns
import typer📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import matplotlib.pyplot as plt | |
| import mesa | |
| from mesa.datacollection import DataCollector | |
| import numpy as np | |
| import polars as pl | |
| import seaborn as sns | |
| import mesa | |
| from mesa.datacollection import DataCollector | |
| import numpy as np | |
| import polars as pl | |
| import typer |
🤖 Prompt for AI Agents
In examples/boltzmann_wealth/backend_mesa.py around lines 11 to 16, remove the
unused imports matplotlib.pyplot and seaborn from the import block; keep only
the modules actually used (mesa, DataCollector, numpy, polars) so the file does
not include unused dependencies and lints cleanly.
| - `--save-results / --no-save-results` Persist CSV metrics | ||
| - `--results-dir` Override the auto timestamped directory under `results/` | ||
|
|
||
| Frames backend additionally warns if runtime type checking is enabled because it | ||
| slows vectorised operations: set `MESA_FRAMES_RUNTIME_TYPECHECKING=0` for fair | ||
| performance comparisons. |
There was a problem hiding this comment.
Minor grammar: hyphenate compound adjective.
Line 57: "auto timestamped" should be "auto-timestamped" (compound adjective modifying "directory").
Apply this diff:
- `--results-dir` Override the auto timestamped directory under `results/`
+ `--results-dir` Override the auto-timestamped directory under `results/`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - `--save-results / --no-save-results` Persist CSV metrics | |
| - `--results-dir` Override the auto timestamped directory under `results/` | |
| Frames backend additionally warns if runtime type checking is enabled because it | |
| slows vectorised operations: set `MESA_FRAMES_RUNTIME_TYPECHECKING=0` for fair | |
| performance comparisons. | |
| - `--save-results / --no-save-results` Persist CSV metrics | |
| - `--results-dir` Override the auto-timestamped directory under `results/` | |
| Frames backend additionally warns if runtime type checking is enabled because it | |
| slows vectorised operations: set `MESA_FRAMES_RUNTIME_TYPECHECKING=0` for fair | |
| performance comparisons. |
🧰 Tools
🪛 LanguageTool
[grammar] ~57-~57: Use a hyphen to join words.
Context: ...rics - --results-dir Override the auto timestamped directory under results/ ...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In examples/boltzmann_wealth/README.md around lines 56 to 61, the phrase "auto
timestamped directory" is missing a hyphen for the compound adjective; change
"auto timestamped directory under `results/`" to "auto-timestamped directory
under `results/`" to correct grammar and hyphenate the compound modifier.
| if save_results: | ||
| result.datacollector.flush() | ||
|
|
||
| if plot: | ||
| # Create a subdirectory for per-metric plots under the timestamped | ||
| # results directory. For each column in the model metrics (except | ||
| # the step index) create a single-metric DataFrame and call the | ||
| # shared plotting helper to export light/dark PNG+SVG variants. | ||
| plots_dir = results_dir / "plots" | ||
| plots_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Determine which columns to plot (preserve 'step' if present). | ||
| value_cols = [c for c in model_metrics.columns if c != "step"] | ||
| for col in value_cols: | ||
| stem = f"{col}_{timestamp}" | ||
| single = ( | ||
| model_metrics.select(["step", col]) | ||
| if "step" in model_metrics.columns | ||
| else model_metrics.select([col]) | ||
| ) | ||
| plot_model_metrics( | ||
| single, | ||
| plots_dir, | ||
| stem, | ||
| title=f"Sugarscape IG — {col.capitalize()}", | ||
| subtitle=f"mesa-frames backend; seed={result.datacollector.seed}", | ||
| agents=agents, | ||
| steps=steps, | ||
| ) | ||
|
|
||
| typer.echo(f"Saved plots under {plots_dir}") | ||
|
|
||
| typer.echo(f"Saved CSV results under {results_dir}") |
There was a problem hiding this comment.
Misleading "Saved CSV results" message when save_results=False.
Line 495 always prints "Saved CSV results under {results_dir}" regardless of whether save_results is True. This could confuse users who disabled CSV persistence.
if save_results:
result.datacollector.flush()
+ typer.echo(f"Saved CSV results under {results_dir}")
if plot:
# Create a subdirectory for per-metric plots under the timestamped
# results directory. For each column in the model metrics (except
# the step index) create a single-metric DataFrame and call the
# shared plotting helper to export light/dark PNG+SVG variants.
plots_dir = results_dir / "plots"
plots_dir.mkdir(parents=True, exist_ok=True)
# Determine which columns to plot (preserve 'step' if present).
value_cols = [c for c in model_metrics.columns if c != "step"]
for col in value_cols:
stem = f"{col}_{timestamp}"
single = (
model_metrics.select(["step", col])
if "step" in model_metrics.columns
else model_metrics.select([col])
)
plot_model_metrics(
single,
plots_dir,
stem,
title=f"Sugarscape IG — {col.capitalize()}",
subtitle=f"mesa-frames backend; seed={result.datacollector.seed}",
agents=agents,
steps=steps,
)
typer.echo(f"Saved plots under {plots_dir}")
-
- typer.echo(f"Saved CSV results under {results_dir}")🤖 Prompt for AI Agents
In examples/sugarscape_ig/backend_frames/model.py around lines 463 to 495, the
final typer.echo always prints "Saved CSV results under {results_dir}" even when
save_results is False; change this so the message is only emitted when CSVs were
actually written. Concretely, wrap or move the typer.echo(f"Saved CSV results
under {results_dir}") inside an if save_results: block (or replace it with a
conditional message that indicates results were not saved), so users only see
the CSV-saved confirmation when save_results is True.
| def __init__( | ||
| self, | ||
| model: Sugarscape, | ||
| *, | ||
| sugar: int, | ||
| metabolism: int, | ||
| vision: int, | ||
| ) -> None: |
There was a problem hiding this comment.
Undefined type Sugarscape will cause runtime errors with beartype.
The type hint model: Sugarscape references a class that is neither imported nor defined in this module. While from __future__ import annotations defers annotation evaluation, this will still fail at runtime if beartype is enabled for type checking (per coding guidelines, TYPE_CHECKING guards are avoided because beartype requires actual type objects).
Consider importing the model class or using a string literal with a proper import:
from typing import Tuple
import mesa
+
+from examples.sugarscape_ig.backend_mesa.model import SugarscapeAlternatively, if a circular import exists, use mesa.Model as the type hint instead:
- def __init__(
- self,
- model: Sugarscape,
+ def __init__(
+ self,
+ model: mesa.Model,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def __init__( | |
| self, | |
| model: Sugarscape, | |
| *, | |
| sugar: int, | |
| metabolism: int, | |
| vision: int, | |
| ) -> None: | |
| def __init__( | |
| self, | |
| model: mesa.Model, | |
| *, | |
| sugar: int, | |
| metabolism: int, | |
| vision: int, | |
| ) -> None: |
🧰 Tools
🪛 Ruff (0.14.7)
21-21: Undefined name Sugarscape
(F821)
🤖 Prompt for AI Agents
In examples/sugarscape_ig/backend_mesa/agents.py around lines 19 to 26, the
constructor type hint uses Sugarscape which is not imported or defined and will
break beartype at runtime; fix by either importing the actual model class (e.g.,
add a top-level import for Sugarscape from its module), or replace the
annotation with a forward-reference string of the fully-qualified class name to
ensure the type object exists at runtime, or if there is a circular import use
the generic mesa.Model type instead; update the file imports accordingly and
ensure the chosen type is available at runtime for beartype.
| seed_val = None | ||
| if "seed" in model_pd.columns and not model_pd.empty: | ||
| seed_val = model_pd["seed"].iloc[0] |
There was a problem hiding this comment.
Remove unused variable seed_val.
Static analysis correctly identified that seed_val is assigned but never used.
- seed_val = None
- if "seed" in model_pd.columns and not model_pd.empty:
- seed_val = model_pd["seed"].iloc[0]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| seed_val = None | |
| if "seed" in model_pd.columns and not model_pd.empty: | |
| seed_val = model_pd["seed"].iloc[0] |
🧰 Tools
🪛 Ruff (0.14.7)
260-260: Local variable seed_val is assigned to but never used
Remove assignment to unused variable seed_val
(F841)
🤖 Prompt for AI Agents
In examples/sugarscape_ig/backend_mesa/model.py around lines 258 to 260, the
local variable `seed_val` is assigned from model_pd but never used; remove the
`seed_val` assignment (or if intent was to use the seed, replace usages to read
directly from model_pd or store it in a class/variable where it's actually
referenced). Ensure no other code expects `seed_val` and run tests/linters to
confirm the unused variable is gone.
| Notes on interpretation: | ||
|
|
||
| - `agents_alive` typically decreases until a quasi steady state (metabolism vs regrowth) or total collapse. | ||
| - `mean_sugar` and `total_sugar` may stabilise if regrowth balances metabolism. | ||
| - Rising `gini` indicates emerging inequality; sustained high values suggest strong positional advantages. | ||
| - Correlations near 0 imply weak linear relationships; positive `corr_sugar_vision` suggests high vision aids resource gathering. Negative `corr_sugar_metabolism` can emerge if high metabolism accelerates starvation. | ||
|
|
There was a problem hiding this comment.
Minor grammar: hyphenate compound adjectives.
Line 50: "quasi steady state" should be "quasi-steady state". Also check for consistency: "high-metabolism agents" (line 53 suggestion) uses a hyphen correctly, so apply the same style.
Apply this diff:
- `agents_alive` typically decreases until a quasi steady state (metabolism vs regrowth) or total collapse.
+ `agents_alive` typically decreases until a quasi-steady state (metabolism vs regrowth) or total collapse.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Notes on interpretation: | |
| - `agents_alive` typically decreases until a quasi steady state (metabolism vs regrowth) or total collapse. | |
| - `mean_sugar` and `total_sugar` may stabilise if regrowth balances metabolism. | |
| - Rising `gini` indicates emerging inequality; sustained high values suggest strong positional advantages. | |
| - Correlations near 0 imply weak linear relationships; positive `corr_sugar_vision` suggests high vision aids resource gathering. Negative `corr_sugar_metabolism` can emerge if high metabolism accelerates starvation. | |
| Notes on interpretation: | |
| - `agents_alive` typically decreases until a quasi-steady state (metabolism vs regrowth) or total collapse. | |
| - `mean_sugar` and `total_sugar` may stabilise if regrowth balances metabolism. | |
| - Rising `gini` indicates emerging inequality; sustained high values suggest strong positional advantages. | |
| - Correlations near 0 imply weak linear relationships; positive `corr_sugar_vision` suggests high vision aids resource gathering. Negative `corr_sugar_metabolism` can emerge if high metabolism accelerates starvation. |
🧰 Tools
🪛 LanguageTool
[grammar] ~50-~50: Use a hyphen to join words.
Context: ...alive` typically decreases until a quasi steady state (metabolism vs regrowth) or...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In examples/sugarscape_ig/README.md around lines 48 to 54, fix hyphenation of
compound adjectives: change "quasi steady state" to "quasi‑steady state" and
ensure consistency by using hyphenation for other compounds as needed (e.g.,
"high-metabolism agents"); update the phrase in-place so all compound adjectives
in this block follow the same hyphenated style.
|
@Ben-geo can you take the lead in reviewing this? |
|
Yep, I’ve got it! - I’ll review all the new PRs this week. I’ll start going through some tonight, and I’ll wrap up by the weekend. |
This PR introduces:
runcommand with standardised options(
--agents,--steps,--seed,--plot,--save-results,--results-dir).boltzmann,sugarscape) with automated CSV output and scaling plots.Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.