feat: Implement Altair version of grid visualization
Demo for Boltzmann wealth model:
https://github.com/projectmesa/mesa/assets/395821/a74aaf4f-1547-41f8-88f5-466500506317
Performance benchmarks:
| Model | Size | Init time [95% CI] | Run time [95% CI] |
|---|---|---|---|
| Schelling | small | 🔵 +2.1% [+1.8%, +2.3%] | 🔵 +0.2% [+0.1%, +0.3%] |
| Schelling | large | 🔵 +1.8% [+1.3%, +2.3%] | 🔵 -0.7% [-1.5%, -0.1%] |
| WolfSheep | small | 🔵 +0.1% [-0.3%, +0.5%] | 🔵 -0.3% [-0.4%, -0.1%] |
| WolfSheep | large | 🔵 +2.7% [-1.2%, +7.9%] | 🔵 +1.5% [-0.8%, +4.1%] |
| BoidFlockers | small | 🔵 -1.6% [-2.0%, -1.2%] | 🔵 +0.4% [-0.2%, +0.9%] |
| BoidFlockers | large | 🔵 +0.1% [-0.5%, +0.5%] | 🔵 +0.9% [+0.3%, +1.5%] |
The animation jitter could be fixed with a plt.tight_layout() equivalent of Altair. I just haven't figured it out yet.
Fixed for multigrid, where now you may see overlapping markers:
Thanks for this effort, will try to review it later today.
Also curious what @Corvince and @ankitk50 think
Nice and simple solution, but the obvious question is how this relates to https://github.com/projectmesa/mesa/pull/1902
This PR has a agent_portrayal function, but I don't see how it is used. Since altair uses a declarative approach vs an imperative in matplotlib, I think its confusing to have different agent_portrayal functions. But would have to see the agent_portrayal.
Otherwise drawing agents on the edge itself looks strange to me, but this shouldn't be a deciding factor.
Since altair uses a declarative approach vs an imperative in matplotlib, I think its confusing to have different agent_portrayal functions. But would have to see the agent_portrayal.
I need to say that there is no modification needed in https://github.com/projectmesa/mesa-examples/blob/21624513c6ad94faabbb1cb4b97d73b897f9afae/examples/boltzmann_wealth_model_experimental/app.py#L5-L11, i.e. the agent_portrayal API is the same for both Matplotlib and Altair version. I just need to specify space_drawer="altair" in the JupyterViz initialization.
Since altair uses a declarative approach vs an imperative in matplotlib, I think its confusing to have different agent_portrayal functions. But would have to see the agent_portrayal.
I need to say that there is no modification needed in https://github.com/projectmesa/mesa-examples/blob/21624513c6ad94faabbb1cb4b97d73b897f9afae/examples/boltzmann_wealth_model_experimental/app.py#L5-L11, i.e. the
agent_portrayalAPI is the same for both Matplotlib and Altair version. I just need to specifyspace_drawer="altair"in theJupyterVizinitialization.
Not exactly. For size this works as you described, but for color in matplotlib you set the actual colors (in your example tab:red and tab:blue ). In altair/vega you set the color encoding. So you are just saying give the value "tab:red" one color and the value "tab:blue" another one. The actual colors that are used are just the default categorical ones (lightblue and orange)
I was about to switch the Altair viz color behavior to match Matplotlib (i.e. color means color, instead of category encoded in color), but decided against it, because the underlying code will look more opaque for the users to modify (edit: as it deviates from idiomatic Altair). As such, I'm leaving the code in this PR as is, to limit its scope. The scope is to have a feature parity with Matplotlib JupyterViz's _draw_grid as much as possible.
We need something like the 2nd point in https://github.com/projectmesa/mesa/pull/1988#issuecomment-1904528049 that @quaquel raised. This can be in the form of abstracting out the chart definition https://github.com/projectmesa/mesa/pull/1991/files#diff-681727772982fb0400d413fc9253bbc5e1af9b51d5a9d8581f2ab9d39a3cd516R39-R51 to a function that can be specified by the user.
The animation jitter could be fixed with a plt.tight_layout() equivalent of Altair. I just haven't figured it out yet.
I fixed this by converting the data from DF to alt.Data.
New version of the video, with no more jitter.
https://github.com/projectmesa/mesa/assets/395821/d0bbaefb-bfa8-43e4-b29e-684bf0ae396f
The only concern in this PR is that the agent_portrayal function differs from the Matplotlib version in that color is categorical instead of literal. I have chosen to keep the discrepancy, to adapt to Altair whenever it makes sense instead.
Are there any other concerns?
I don't know why, but I can't view the movie.
I have converted the movie output from .mp4 to .gif
Can we move forward with this PR ASAP? I want to implement #2049 but for Altair and many other things. There is nothing controversial in this PR that warrants a delay in review for a month. And I'm not pleased with the slowdown in the supposedly experimental feature.
Can we move forward with this PR ASAP? I want to implement #2049 but for Altair and many other things. There is nothing controversial in this PR that warrants a delay in review for a month. And I'm not pleased with the slowdown in the supposedly experimental feature.
I understand your frustration, but for me the main question is still and you haven't commented on this yet, how this relates to #1902 and how to deal with this situation, as I asked on Matrix. I think this is very controversial
There have been various requests for modifications to the code in #1902. The last response from the author of that PR is from 2 weeks ago and in no way acknowledges the requested changes. I am sympathetic to therefore moving forward with this one instead.
I understand your frustration, but for me the main question is still and you haven't commented on this yet, how this relates to https://github.com/projectmesa/mesa/pull/1902 and how to deal with this situation, as I asked on Matrix. I think this is very controversial
@Corvince I already replied almost immediately on your question on Matrix, on Matrix, more than a week ago.
I really don't know what to do with the two competing jupyter PRs implementing the same feature. How can we resolve this without devaluing the work of one of the PRs?
I guess this is @Corvince's question. I looked through your responses @rht, but I don't see a clear answer to this specific question. You did write
in #1991 i attribute @rlskoeser because that's what it is mainly based on, and we had already made a fanfare about including her in the next mesa release (https://github.com/projectmesa/mesa/pull/1991#discussion_r1463378129). i can't exactly attribute the student (ankit) either, because his pr is mainly based on corvince's mesa-interactive, almost an exact copy. i'm not sure if ankit intentionally ignored my reviews or something else. https://github.com/projectmesa/mesa/pull/1820 seems to suggest the former. it's meant to be a bugfix, where i have outlined a suggestion with a very simple fix (https://github.com/projectmesa/mesa/pull/1820#issuecomment-1774838319), but hasn't been responded to for ages. if i shouldn't be frustrated about this, i don't know what else.
So, are you saying that your answer to @Corvince is to just merge this PR and close the other without any acknowledgement to the author of #1902?
So, are you saying that your answer to @Corvince is to just merge this PR and close the other without any acknowledgement to the author of https://github.com/projectmesa/mesa/pull/1902?
Yes, mesa-interactive already exists in a separate repo, https://github.com/Corvince/mesa-interactive.
I don't know the backstory to any of this. At face value, merging this PR without any acknowledgement to the author of #1902 seems a s bit harsh even though the author has not replied to various reasonable modifications to the code. What is wrong with a simple mention in the PR message itself of the work done in #1902?
Thank you for moving this forward. Finally.