Moving examples to use the new discrete spaces #198

quaquel · 2024-09-10T15:29:51Z

This PR moves over all relevant examples to the experimental grid spaces. While making this move, the experimental grid spaces continue to be refined.

EwoutH · 2024-09-13T09:51:40Z

If you change this line:

mesa-examples/.github/workflows/test_examples.yml

Line 58 in 2b43544

pip install -U git+https://github.com/projectmesa/mesa@main#egg=mesa

To:

pip install -U git+https://github.com/quaquel/mesa@move_spaces#egg=mesa

You can see if it works in CI.

quaquel · 2024-09-13T10:10:57Z

I am testing locally against my branch for #2286, but I'll update the the yml in this branch to show CI here as well.

EwoutH · 2024-09-26T10:22:49Z

This PR just need the imports switched right?

quaquel · 2024-09-26T10:47:06Z

The smart thing to do would be to change the imports to experimental.cell_space. This would make this pr compatible with main.

EwoutH · 2024-09-26T11:05:53Z

Awesome, sounds like a plan!

EwoutH · 2024-10-07T07:42:18Z

If you need anything on this PR let me know!

quaquel · 2024-10-07T07:54:20Z

If you need anything on this PR let me know!

How to get the build to succeed? It now errors out on reaction?

Other than that, I think this is another half our of work later today and then its done.

EwoutH · 2024-10-07T08:19:09Z

Awesome progress!

How to get the build to succeed? It now errors out on reaction?

I think the error is this init (https://github.com/projectmesa/mesa/blob/main/mesa/experimental/__init__.py) in combination with that we now don’t install Solara by default anymore. Let me look into it.

EwoutH

I really like how clean the API is looking, and how easy old components are replaced. Good stuff!

EwoutH · 2024-10-12T12:18:58Z

examples/hotelling_law/hotelling_law/model.py

            store.market_share = 0

-        for consumer in self.consumer_agents:
+        for consumer in  self.agents_by_type[ConsumerAgent]:


Is there a space too much here?

I'll run ruff once I am done and fix it all

quaquel · 2024-10-12T12:31:26Z

I really like how clean the API is looking, and how easy old components are replaced. Good stuff!

I am also pleasantly surprised by (1) how easy the switch is; and (2) how, in many places, it reduces code complexity.

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

EwoutH · 2024-10-12T18:34:28Z

Really awesome, I see some really neat changes in here.

Curious, did you encounter any limitations or go any ideas to do things more elegantly in the cell space?

Let's discuss Wednesday how we want to structure the examples for experimental features. Would be awesome if we can stabilize those together with features in future PRs, once the core examples are moved back.

quaquel · 2024-10-12T18:52:29Z

Curious, did you encounter any limitations or go any ideas to do things more elegantly in the cell space?

I have made various PRs related to grid spaces while working on this. For example, the way we resolved movement #2333 and #2296, the split of neighborhood and get_neigborhood #2309, and the width/height properties #2348 all emerged out of making these examples work with experimental grid spaces. Some of these examples can be refined even further by using property layers (e.g., sugarscape) or using some of the features introduced in the aforementioned PRs. But I am quite happy with how expressive and complete the grid spaces are.

There is also room for further improvement:

We have FixedAgent and CellAgent. I agree with @EwoutH that CellAgent might need a better name. It currently is not a good opposition to FixedAgent. However, I would like to try my hand at mimicking #2296 for Continous Spaces and see what we get there.
I would like to address #2221. In many places, I pass the cell to the init of the agent, but then you can funny code were you instantiate an agent and don't do anything with it. Having a simple factory method to create all agents in one go would be be some much cleaner API wise. Compare the code below:

# current
for cell in self.grid.all_cells:
    MyAgent(self, cell)


# with some create method
MyAgent.create_agents(cell=self.grid.all_cells)

At the moment. CellCollection.agents returns a list of agents. This is primarily because of performance reasons. But API wise it would be nicer if this became an AgentSet. It is quite common in many models to get the neighboring agents and then select one at random or have some selection criterion. If CellCollection.agents would return an agentset, most models would become even shorter and more readable. Just check the the sugarscape_g1mt Trader agent for an example and image that you could use AgentSet functionality in the move and trade method.

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

quaquel · 2024-10-14T18:09:47Z

I have updated Schelling after #222. @EwoutH any further comments or can this be approved?

EwoutH · 2024-10-14T19:17:51Z

Could you split of the changes in the "basic" (as in projectmesa/mesa#2349 (comment)) examples into a new, separate branch and save that somewhere where you can find it back in a few weeks/months? Then we will merge those during/after cell space stabilization.

All "complex" and "showcase" examples can stay in this PR, then we will merge that ASAP.

Sorry it's a bit of a messy process, your effort is appreciated, and it looks like we now have a clear policy for experimental features in examples.

quaquel · 2024-10-15T05:53:26Z

Could you split of the changes in the "basic" (as in projectmesa/mesa#2349 (comment)) examples into a new, separate branch and save that somewhere where you can find it back in a few weeks/months? Then we will merge those during/after cell space stabilization.

It will be annoying but doable if I have time in a few days. Would it not be simpler to move the basis versions over to that PR? If I am not mistaken, only Schelling is missing at the moment. Next, we merge this, continue the move of advanced examples and once that PR is merged remove all moved over examples from mesa-examples.

Mainly imports

Revert the basic models back to the current stable grids

EwoutH · 2024-10-15T06:39:33Z

For a proper port of the git history its way easier if we can move the examples at once, otherwise we're merging multiple related git histories which makes it even more complex.

I reverted the basic examples, and saved the changes in a backup branch: https://github.com/projectmesa/mesa-examples/tree/pr_198_examples_backup

projectmesa/mesa#2357 should unblock the CI and then we're good to go.

quaquel · 2024-10-15T06:44:43Z

For a proper port of the git history its way easier if we can move the examples at once, otherwise we're merging multiple related git histories which makes it even more complex.

Fair point. I had not considered that.

EwoutH · 2024-10-15T06:52:32Z

Main CI passes, awesome.

Thanks a lot for this huge effort!

EwoutH

I was a bit enthosiastic on my approval, only after realizing I didn't do a proper code review yet.

Overal, it looks amazing, with some really nice cleanups and elegant constructs! I have a few small things, then can (but should) be dealt with in a follow-up PR.

Especially check if if all your changes were ported correctly after #220, and then remove the duplicate files.

EwoutH · 2024-10-15T06:53:50Z

examples/aco_tsp/aco_tsp/model.py

@@ -93,6 +94,7 @@ def __init__(self, model, alpha: float = 1.0, beta: float = 5.0):
        self._traveled_distance = 0
        self.tsp_solution = []
        self.tsp_distance = 0
+        self.graph = self.model.grid.G


Would it be useful to make this a property or function?

not sure what you are proposing. This is in the agent class.

EwoutH · 2024-10-15T06:55:14Z

examples/aco_tsp/aco_tsp/model.py

@@ -173,14 +180,15 @@ def __init__(
        self.num_cities = tsp_graph.num_cities
        self.all_cities = set(range(self.num_cities))
        self.max_steps = max_steps
-        self.grid = mesa.space.NetworkGrid(tsp_graph.g)
+        self.grid = Network(tsp_graph.g, random=self.random)


Maybe call it space here, since it isn't really a grid?

Suggested change

self.grid = Network(tsp_graph.g, random=self.random)

self.space = Network(tsp_graph.g, random=self.random)

EwoutH · 2024-10-15T06:56:00Z

examples/bank_reserves/BankReservesModel_Data.csv

Do we really want this data file included?

not sure, but the entire batch run example stuff seems incorrect so this needs a bit more attention

examples/aco_tsp/aco_tsp/model.py

examples/bank_reserves/bank_reserves/random_walk.py

examples/hotelling_law/hotelling_law/agents.py

examples/hotelling_law/hotelling_law/model.py

EwoutH · 2024-10-15T07:18:13Z

examples/schelling_experimental/model.py

Can be removed I think

examples/shape_example/shape_example/model.py

quaquel · 2024-10-15T07:46:51Z

Thanks for these comments. I just like to point out that most of these are comments on the original code from which I started, not the changes made in this PR. Some of the other comments are about using features that were added based on what was learned while doing this. I'll try to go through these in follow up PRs when I have time. A quick look suggests only 1 is pointing out a mistake in my code.

EwoutH · 2024-10-15T08:00:24Z

Oh certainly, and none of them were blocking. Just things I noticed when going through the diff.

Do you prefer to fix some of them before the example move, or after?

EwoutH mentioned this pull request Oct 9, 2024

Fix __all__ imports in __init__.pys projectmesa/mesa#2343

Open

EwoutH reviewed Oct 12, 2024

View reviewed changes

quaquel requested a review from EwoutH October 12, 2024 13:04

quaquel and others added 18 commits October 12, 2024 20:09

moving acto_tsp

41b2dd1

Update model.py

e775b42

move bank model to new OrthogonalMooreGrid

1b629a6

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

a1a15b3

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

fix random in atc

bd0a4fa

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

bab2de8

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

moved all boltman models over

a0d40a4

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

9b89085

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

another batch of models

9b833f4

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

ef61c59

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

bring in line with main

281e9db

figuring out build failure

ca2317f

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

e88d645

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

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

a94c325

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

2 more examples moved

7fd5774

updated hotelling

4e99603

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

28a348b

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

add seed kwarg

878b51a

pre-commit-ci bot and others added 4 commits October 12, 2024 20:10

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

d57aa74

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

ruff related fixes

480cdb4

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

c433382

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

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

40399c7

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

quaquel force-pushed the new_spaces branch from e37eeb9 to 40399c7 Compare October 12, 2024 18:10

quaquel and others added 2 commits October 12, 2024 20:26

rebase and some additional fixes

86f8592

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

e5c69b6

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

This was referenced Oct 13, 2024

Move core example model back projectmesa/mesa#2349

Closed

refactor: Simplify Schelling code #222

Merged

quaquel and others added 2 commits October 14, 2024 20:07

Merge remote-tracking branch 'upstream/main' into new_spaces

95f862e

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

f9f7b84

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

EwoutH added 2 commits October 15, 2024 08:18

Fix pre-commit

acbff93

Mainly imports

Revert basic example models to current grids

6b804bd

Revert the basic models back to the current stable grids

EwoutH mentioned this pull request Oct 15, 2024

experimental init: Fix Solara import by making it lazy projectmesa/mesa#2357

Merged

EwoutH approved these changes Oct 15, 2024

View reviewed changes

quaquel merged commit 9a05b05 into projectmesa:main Oct 15, 2024
2 of 3 checks passed

EwoutH reviewed Oct 15, 2024

View reviewed changes

quaquel mentioned this pull request Oct 15, 2024

bug fix and code simplification in hotelling #223

Merged

EwoutH mentioned this pull request Oct 15, 2024

Remove duplicate files #224

Merged

quaquel deleted the new_spaces branch November 11, 2024 18:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moving examples to use the new discrete spaces #198

Moving examples to use the new discrete spaces #198

quaquel commented Sep 10, 2024 •

edited

Loading

EwoutH commented Sep 13, 2024

quaquel commented Sep 13, 2024

EwoutH commented Sep 26, 2024

quaquel commented Sep 26, 2024

EwoutH commented Sep 26, 2024

EwoutH commented Oct 7, 2024

quaquel commented Oct 7, 2024

EwoutH commented Oct 7, 2024 •

edited

Loading

EwoutH left a comment

EwoutH Oct 12, 2024

quaquel Oct 12, 2024

quaquel commented Oct 12, 2024

EwoutH commented Oct 12, 2024

quaquel commented Oct 12, 2024 •

edited

Loading

quaquel commented Oct 14, 2024

EwoutH commented Oct 14, 2024

quaquel commented Oct 15, 2024

EwoutH commented Oct 15, 2024

quaquel commented Oct 15, 2024

EwoutH commented Oct 15, 2024

EwoutH left a comment

EwoutH Oct 15, 2024

quaquel Oct 15, 2024

EwoutH Oct 15, 2024

EwoutH Oct 15, 2024

quaquel Oct 15, 2024

EwoutH Oct 15, 2024

quaquel Oct 15, 2024

quaquel commented Oct 15, 2024

EwoutH commented Oct 15, 2024

	self.grid = Network(tsp_graph.g, random=self.random)
	self.space = Network(tsp_graph.g, random=self.random)

Moving examples to use the new discrete spaces #198

Moving examples to use the new discrete spaces #198

Conversation

quaquel commented Sep 10, 2024 • edited Loading

EwoutH commented Sep 13, 2024

quaquel commented Sep 13, 2024

EwoutH commented Sep 26, 2024

quaquel commented Sep 26, 2024

EwoutH commented Sep 26, 2024

EwoutH commented Oct 7, 2024

quaquel commented Oct 7, 2024

EwoutH commented Oct 7, 2024 • edited Loading

EwoutH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quaquel commented Oct 12, 2024

EwoutH commented Oct 12, 2024

quaquel commented Oct 12, 2024 • edited Loading

quaquel commented Oct 14, 2024

EwoutH commented Oct 14, 2024

quaquel commented Oct 15, 2024

EwoutH commented Oct 15, 2024

quaquel commented Oct 15, 2024

EwoutH commented Oct 15, 2024

EwoutH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quaquel commented Oct 15, 2024

EwoutH commented Oct 15, 2024

quaquel commented Sep 10, 2024 •

edited

Loading

EwoutH commented Oct 7, 2024 •

edited

Loading

quaquel commented Oct 12, 2024 •

edited

Loading