mesa-examples icon indicating copy to clipboard operation
mesa-examples copied to clipboard

Moving examples to use the new discrete spaces

Open quaquel opened this issue 1 year ago • 2 comments

This PR is WIP and tied to #2286.

I am moving all examples that use a network, grid, or hexgrid over to the currently still experimental discrete spaces. In this process, we can also see if the API is mature enough or needs further development. I track identified issues that warrant attention in the MESA PR. Only after #2286 has been merged can we also merge this PR.

quaquel avatar Sep 10 '24 15:09 quaquel

If you change this line: https://github.com/projectmesa/mesa-examples/blob/2b4354481c6f0341ced94526f29035840d326adb/.github/workflows/test_examples.yml#L58

To:

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

You can see if it works in CI.

EwoutH avatar Sep 13 '24 09:09 EwoutH

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.

quaquel avatar Sep 13 '24 10:09 quaquel

This PR just need the imports switched right?

EwoutH avatar Sep 26 '24 10:09 EwoutH

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

quaquel avatar Sep 26 '24 10:09 quaquel

Awesome, sounds like a plan!

EwoutH avatar Sep 26 '24 11:09 EwoutH

If you need anything on this PR let me know!

EwoutH avatar Oct 07 '24 07:10 EwoutH

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.

quaquel avatar Oct 07 '24 07:10 quaquel

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 avatar Oct 07 '24 08:10 EwoutH

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.

quaquel avatar Oct 12 '24 12:10 quaquel

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.

EwoutH avatar Oct 12 '24 18:10 EwoutH

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.

quaquel avatar Oct 12 '24 18:10 quaquel

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

quaquel avatar Oct 14 '24 18:10 quaquel

Could you split of the changes in the "basic" (as in https://github.com/projectmesa/mesa/pull/2349#issuecomment-2408945055) 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.

EwoutH avatar Oct 14 '24 19:10 EwoutH

Could you split of the changes in the "basic" (as in https://github.com/projectmesa/mesa/pull/2349#issuecomment-2408945055) 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.

quaquel avatar Oct 15 '24 05:10 quaquel

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

https://github.com/projectmesa/mesa/pull/2357 should unblock the CI and then we're good to go.

EwoutH avatar Oct 15 '24 06:10 EwoutH

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.

quaquel avatar Oct 15 '24 06:10 quaquel

Main CI passes, awesome.

Thanks a lot for this huge effort!

EwoutH avatar Oct 15 '24 06:10 EwoutH

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.

quaquel avatar Oct 15 '24 07:10 quaquel

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 avatar Oct 15 '24 08:10 EwoutH