-
Notifications
You must be signed in to change notification settings - Fork 858
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
Voronoi Tesselation based Discrete Space #2084
base: main
Are you sure you want to change the base?
Conversation
Thanks, it's a great start! Using DelaunayTri is a smart approach. I guess Another thing that it's needed in the long term is a method to check in which cell a point (and thus an agent) is. @wang-boyu I would also appreciate a review from your mesa-geo expertise. For the currently functionality, I would like some additional docstring and unittests. Then you can decide if you want to add further functionality to this PR, or merge this first and follow up with new PRs. Edit: One thought, also for the other maintainers: Would we need separate Discrete and Continuous Voronoi spaces? |
class VoronoiGrid(DiscreteSpace): | ||
def __init__( | ||
self, | ||
centroids_coordinates: Sequence[Sequence[float]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this work with Shapely Points? That might be very useful for Mesa geo applications (cc @wang-boyu).
Just add with adding a simple test case for this, maybe it will just work.
|
||
|
||
class VoronoiGrid(DiscreteSpace): | ||
def __init__( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's important to to initialize the super class here (with super().__init__()
)
from random import Random | ||
from typing import Optional | ||
|
||
from pyhull.delaunay import DelaunayTri |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit worried about adding this package as a dependency. It's last update was in 2015.
Maybe we can use SciPy?
- https://docs.scipy.org/doc/scipy/reference/generated/scipy.spatial.Delaunay.html
- https://docs.scipy.org/doc/scipy/tutorial/spatial.html#delaunay-triangulations
Otherwise maybe shapely.ops.triangulate
or keeping the code in our own repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, I choose PyHull because Mesa is not using Scipy already and it is a very heavy dependency. I'll take a look at Shapely, it looks a good alternative, and as you said, can be integrated with Mesa Geo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the context. Shapely is also quite heavy unfortunately. It’s just a little code, can we house it in an own class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The computation can be quite heavy acoording to the number of points, but it is duable. In my opinion, since the result of Delaunay Triangulation is a graph, we can use NetworkGrid to represent it, and in the continuous case, implement in the Mesa Geo using Shapely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable!
Thanks for the PR. I have to say that I'm still a bit confused about why this is based on discrete space, not continuous space (as I mentioned in #1895 (comment) and #1895 (comment)). As a concrete example, consider this simple space:
If it is a Voronoi space, should Cell 2 be split into halves, where left half belongs to agent of type O and right half belongs to agent of type X? How does this work in discrete space? |
Apologies, a little late to this discussion, this is really cool @vitorfrois. As dependencies and keeping Mesa lightweight is a constant struggle, but form a very superficial look is there a reason we wont use networkx (https://networkx.org/documentation/stable/reference/algorithms/voronoi.html) and this has a corresponding example that may work for mesa_geo (https://networkx.org/documentation/stable/auto_examples/geospatial/plot_delaunay.html)? Let me know what you think/ what I am missing? |
I'm considering the discrete space for Voronoi Tesselation as a static/non changing graph, where neighbors are obtained by Delaunay Triangulation. As @EwoutH said, it is only a special case for HexGrid where cell are not equally spaced. I think its more about a choice of implementation where I considered mainly the Cholera example. As I suggested yesterday,
Does that make sense? |
@tpike3 thanks for the comment. That's more or less what I'm thinking. In Mesa Geo, we have opportunity to do it better with Shapely |
Thanks for clarifying. Appreciate it. Question: Is the Delaunay Triangulation done in continuous space? |
Yes, it is |
While it might be desired in a specific model to have the same agent be placed in multiple spots simultaneously, the typical use case is that one agent has one position at every given moment. This commit decorates the place_agent() method in a way that it emits a warning when called with an agent which already has a location. Fixes: projectmesa#1522
uv is a fast drop-in pip, pip-compile, virtualenv etc replacement created by the creator of Ruff.
* Update Ruff to 0.3.4; apply ruff format . * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…ojectmesa#2087) This fixes - some minor grammatical errors, - an incorrect header indentation level,
updates: - [github.com/astral-sh/ruff-pre-commit: v0.3.4 → v0.3.5](astral-sh/ruff-pre-commit@v0.3.4...v0.3.5) - [github.com/asottile/pyupgrade: v3.15.0 → v3.15.2](asottile/pyupgrade@v3.15.0...v3.15.2)
for more information, see https://pre-commit.ci
updates: - [github.com/astral-sh/ruff-pre-commit: v0.4.3 → v0.5.0](astral-sh/ruff-pre-commit@v0.4.3...v0.5.0) - [github.com/asottile/pyupgrade: v3.15.2 → v3.16.0](asottile/pyupgrade@v3.15.2...v3.16.0) - [github.com/codespell-project/codespell: v2.2.6 → v2.3.0](codespell-project/codespell@v2.2.6...v2.3.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This doesn't seem to be used as often as the current simple namespace.
Fixes two things pre-commit detected: - Some codespell errors - A case of E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
Removing the old, legacy visualisation. It will still be available in 2.3.x. Checkout the Visualization Tutorial (https://mesa.readthedocs.io/en/latest/tutorials/visualization_tutorial.html) to migrate to the new visualisation.
Currently measures crashed when it's None (which is the default). The only way to circumvent this is adding [], but that's non-ideal and not obvious. This PR fixes that, so that measures can actually be the default value of None. This also proves we need to test Jupyter viz in notebooks, not only in console, since both have different code paths.
- Add docstring to Jupyter viz module and functions - Render the docstring in Read the Docs: https://mesa.readthedocs.io/en/latest/apis/visualization.html
* Jupyter Viz: Don't avoid interactive backend We were avoiding the interactive backend, but that isn't recommended anymore and Solara should take care of that itself. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sets the version to 3.0.0a0 for the first Mesa 3.0 pre-release, and updates the release notes with 2.3.1 and 3.0.0a0 changelog
Don't track virtual environment files in Git
for more information, see https://pre-commit.ci
Updates
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really outstanding, Vítor. That space visualization especially, it really offers a lot of value.
Small minor comments. I would also like to see more test coverage, if you have the chance.
I’m going to approve conceptually, especially considering the cell space is still experimental. @rht are you able to do a technical code review and merge if it looks good?
CC @quaquel, you might also find this interesting (and have some comments).
@@ -43,6 +43,7 @@ dependencies = [ | |||
"pandas", | |||
"solara", | |||
"tqdm", | |||
"pyhull", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can now be removed right?
from mesa.experimental.cell_space.discrete_space import DiscreteSpace | ||
|
||
|
||
class Delaunay: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we put this in a separate file? @rht?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In voronoi.py should be fine. We can split it once it is used in more than one context. The docstring should clearly indicate that this is a helper class, and not a space implementation it itself. I'd rename it to _Delaunay
for now so that we can merge this PR sooner. We can discuss about other use cases later.
class Delaunay: | ||
""" | ||
Class to compute a Delaunay triangulation in 2D | ||
ref: https://github.com/jmespadero/pyDelaunay2D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How much percentage of the code is based on this GitHub repo? Their license is GPLv3, and we actually can't copy their code because there will be a licensing issue.
Omg, sorry I did not notice the license. I copied like 70% of it's
implementation, changing variables, function names and other minor things.
…On Sun, Jul 21, 2024, 08:44 rht ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In mesa/experimental/cell_space/voronoi.py
<#2084 (comment)>:
> @@ -0,0 +1,264 @@
+from collections.abc import Sequence
+from itertools import combinations
+from random import Random
+
+import numpy as np
+
+from mesa.experimental.cell_space.cell import Cell
+from mesa.experimental.cell_space.discrete_space import DiscreteSpace
+
+
+class Delaunay:
+ """
+ Class to compute a Delaunay triangulation in 2D
+ ref: https://github.com/jmespadero/pyDelaunay2D
How much percentage of the code is based on this GitHub repo? Their
license is GPLv3, and we actually can't copy their code because there will
be a licensing issue.
—
Reply to this email directly, view it on GitHub
<#2084 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALBWUBC63DYQCEJC5TPSGOLZNONJHAVCNFSM6AAAAABFAPRYMCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCOJQGMYDSMZXGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There are 2 ways to proceed:
I'm more in favor of option 2, because option 1 is rather shady. |
Yeah the license issue is a big one. Another (far fetched) option is to contact the maintainer to see if he’s willing to publish on another, more permissive license. |
@vitorfrois I would like to try to contact the maintainers. Do you want to write them a message or would you like me to do it? |
I can do it tomorrow, then hopefully we can move this forward next week! I would like to include it in the next 3.0 alpha release. |
I reached out Tuesday, hopefully we will get an answer soon: jmespadero/pyDelaunay2D#7 |
First version of Voronoi Tesselation based Discrete Space,
described on issue #1895 . This feature allows the user to build a discrete space based on a random sample of points, where neighbors are defined by Delaunay Triangulation.
More specifically, Delaunay Triangulation is a dual-graph representation of the Voronoi Tesselation. Using this algorithm, we can easily find nearest neighbors without delimiting cells edges.
The library chosen for the triangulation algorithm implementation was PyHull, a wrapper of qhull C library, which deals with spatial calculations. The choice was made considering performance, size and usability (SciPy has a similar module but much heavier).
Based on my discussion with @EwoutH on the issue thread, i thought it would be useful to inherit DiscreteSpace class.
Example
Users can input cells centroids
and cells neighborhoods are defined by Delaunay triangulation
Next Steps
Definitely, next steps involve computing the boundaries of each cell using Voronoi Tesselation. This allow us to define capacity of each cell based on its area/volume.