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

Cholera Voronoi #118

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

vitorfrois
Copy link

@vitorfrois vitorfrois commented Apr 7, 2024

Implements agents, model and server graph visualization according to #112 issue.

This example uses the VoronoiGrid, which inherits from the DiscreteSpace, as proposed in projectmesa/mesa#2084.

@EwoutH
Copy link
Member

EwoutH commented Apr 8, 2024

Thanks for the PR! I will try to review it later this week.

@EwoutH EwoutH requested review from rht and EwoutH April 8, 2024 08:18
@EwoutH
Copy link
Member

EwoutH commented Jul 4, 2024

Hi Vítor! I'm really sorry, we really dropped the ball here. The example looks great, and is really useful. I would like to have it in, there are two points that would be nice to have:

Are you interested in picking one or both up? Otherwise I could do it.

Sorry again, it's really appreciated!

@vitorfrois
Copy link
Author

Hello Ewout. This semester was crazy and I got very busy. I'm finally at vacation and expect to implement both tasks.

@EwoutH
Copy link
Member

EwoutH commented Jul 4, 2024

No worries, I know how it goes 😅.

Thanks, looking forward to it! If I can help let me know.

@vitorfrois
Copy link
Author

It looks like this using the new visualization: image. The simulation values are not good as I wish, but I think it is a good example since its using different modules and components

@EwoutH
Copy link
Member

EwoutH commented Jul 10, 2024

Looks great! I will review in the morning.

(I think the import should still be .agents)

Copy link
Member

@EwoutH EwoutH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, a few small comments already. In the morning I will fire it up for a more thorough review.

@quaquel you might also be interested in reviewing this, since it’s one of the first external implementations of the experimental cell space.


The model has two agents: people and pumps. Pumps can infect people and neighbor pumps. People start as susceptible, can be infected by pumps and recover or die, according to a simple SIR model. Each cell has only one pump and is connected to neighbor cells according to Voronoi's diagram. The model aims to investigate how fast actions oriented by Voronoi diagrams can prevent disease spread.

## How to Run
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for writing a Readme! Don’t forget to update this section

self.recovery_chance = recovery_chance

def step(self):
if self.state == REMOVED:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a hunch this whole section could be simpler. Let me check tomorrow.

people = [
obj
for obj in self.cell.agents
if isinstance(obj, Person) and obj.state is not REMOVED
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn’t agents be just removed from cell.agents instead of having a state “removed”?

examples/cholera_voronoi/agents.py Outdated Show resolved Hide resolved
self.model.infected_pumps += 1

# If cases in total is too high, fix pump
cases = sum(1 for a in people if a.state is INFECTIOUS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do len() instead of sum()?

@EwoutH
Copy link
Member

EwoutH commented Jul 10, 2024

@rht @Corvince (and maybe even @maartenbreddels) do you think we can display the cell space easily in Solara? And in this case the actual Vonoroi space? With some color scale depending on the number of infected agents per cell?

@maartenbreddels
Copy link

Short answer, yes: https://py.cafe/maartenbreddels/solara-ipyreact-voronoi :)

I guess you want to customize it a bit, but that's just some fiddling with js examples.

Preview:
preview

@EwoutH
Copy link
Member

EwoutH commented Sep 1, 2024

@vitorfrois could you resolve the comments above if you have already answered them? Then we keep a nice overview what's done and not.


cell_population = [400] * 8

points = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be called pump_locations for clarity?

To run the model interactively, run ``mesa runserver`` in this directory. e.g.

```
$ mesa runserver
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation is outdated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the old visualization, whereas it should have bin solara run run.py.

@rht
Copy link
Contributor

rht commented Sep 8, 2024

Other than the comments, everything else LGTM. The Voronoi visualization can be deferred to a separate PR, since this PR has been hanging in limbo for a while. @maartenbreddels's demo uses D3, but maybe for now we could use Matplotlib, and still be pure Python? There is https://docs.scipy.org/doc/scipy/reference/generated/scipy.spatial.voronoi_plot_2d.html. There is also a way to do it for Altair.

@vitorfrois
Copy link
Author

I will update this example with the visualization for voronoi space included in projectmesa/mesa#1895.

@vitorfrois
Copy link
Author

@EwoutH @rht i did not fully understand the use of the new ABM & DEVSimulator. Should I've been using one of them here?

@EwoutH
Copy link
Member

EwoutH commented Sep 10, 2024

Awesome, looking forward to it! Note that we just updated the visualisation tutorial to the new API.

i did not fully understand the use of the new ABM & DEVSimulator. Should I've been using one of them here?

No, I don't think it's needed here. It's mostly useful if you want to do things outside discrete timesteps.

90% of models don't need DEVS. For the 10% that do it's a total game changer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants