Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make experimental cell spaces work with PropertyGrid #2068

Closed

Conversation

haleelsada
Copy link

@haleelsada haleelsada commented Mar 4, 2024

This PR marks the initial stride towards addressing the issue of integrating experimental cell spaces with PropertyGrid functionality #2059. I have added the PropertyGrid class and kept two methods remove_property_layer, add_property_layer and improved select_random_empty_cell from parent class in experimental.cell_space.grid.py .
The submitted changes represent the initial phase of the requested updates, and I am eager to refine them further based on any feedback received to ensure completion of the necessary modifications.

Sample code:

from mesa import Model
from mesa.experimental.cell_space import OrthogonalMooreGrid, CellAgent
from mesa.space import PropertyLayer


class MyAgent(CellAgent):
    def __init__(self, unique_id, model):
        super().__init__(unique_id, model)


# Initialize a model and grid
model = Model()
grid = OrthogonalMooreGrid(
    [10, 10], torus=True, capacity=1, random=model.random)

# add property layer
property_layer = PropertyLayer("weight", 10, 10, default_value=1)
grid.add_property_layer(property_layer)


# Create an agent and add it to a cell
agent = MyAgent(1, model)
cell = grid.select_random_empty_cell()
agent.move_to(cell)

# get the coordinates and weight layer of the cell
print(cell.coordinate)
print(cell.properties['weight'])

@EwoutH EwoutH requested review from quaquel and Corvince March 4, 2024 14:59
@EwoutH EwoutH added the experimental Release notes label label Mar 4, 2024
Copy link

github-actions bot commented Mar 4, 2024

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
Schelling small 🔵 +0.7% [+0.3%, +1.1%] 🔵 +1.1% [+0.9%, +1.3%]
Schelling large 🔵 +0.9% [+0.0%, +1.7%] 🔴 +9.2% [+6.3%, +12.0%]
WolfSheep small 🔵 +1.5% [-0.3%, +3.2%] 🔵 +1.0% [+0.8%, +1.2%]
WolfSheep large 🔵 +2.6% [+1.6%, +3.5%] 🔴 +13.5% [+10.8%, +15.9%]
BoidFlockers small 🔴 +4.4% [+3.9%, +4.8%] 🔴 +3.6% [+3.2%, +4.1%]
BoidFlockers large 🔵 +3.9% [+3.0%, +4.8%] 🔵 +2.7% [+2.1%, +3.3%]

@EwoutH
Copy link
Member

EwoutH commented Mar 5, 2024

Thanks for the PR! I'm quite busy, I hope to be able to review it this weekend.

@Corvince
Copy link
Contributor

Corvince commented Mar 6, 2024

Thanks for this PR! It looks quite good, I just think the link between cells and the properties should be established directly when a property layer is added (and removed if it is removed). So you don't need to implement another select_random_empty_cell method.

Otherwise some tests would be great, but not strictly required for this to be merged.

removed method select_random_empty_cell from new _PropertyGrid in cell space
@EwoutH
Copy link
Member

EwoutH commented Mar 9, 2024

Starts looking good! I would like to have some unittests in here: https://github.com/projectmesa/mesa/blob/main/tests/test_cell_space.py

You can probably recycle some of these tests:

class TestSingleGridWithPropertyGrid(unittest.TestCase):

@EwoutH
Copy link
Member

EwoutH commented Mar 13, 2024

@haleelsada would you be able to write the unittests?

@tpike3
Copy link
Member

tpike3 commented Mar 17, 2024

@haleelsada would you be able to write the unittests?

@EwoutH Thanks for keeping up with this!

@EwoutH
Copy link
Member

EwoutH commented Mar 17, 2024

@haleelsada are you okay if I write the unittests and add them to this PR? Or do you still want to do this?

@tpike3 No problem! Could you maybe keep a bit track of the visualisation direction and PRs? I am not that familiar with that part of the codebase.

@quaquel quaquel closed this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Release notes label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants