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

More parameter functionality in matplotlib (default) drawer visualization #2242

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

rmhopkins4
Copy link
Contributor

@rmhopkins4 rmhopkins4 commented Aug 23, 2024

(screenshots of changes w/ example agent_portrayal functions coming)

Stuff like this is now possible:

def agent_portrayal(cell):

    return {
        "c": "white" if cell.isAlive else "black",
        "s": 45,
        "marker": "*",
        "edgecolors": "purple",
        "linewidths": 0.5,
        "alpha": 0.5,
    }

image

  • updates necessary docstring
  • supports all params in matplotlib's scatter with the exception of plotnonfinite
  • manual colormapping to get around restrictions similar to those with 'marker', where a unique colormap cannot usually be provided for each point plotted
  • streamlined code for _split_and_scatter and portray

No longer are shape, color, and marker explicitly implemented.
Instead, the implementation is more parameter-independent.
old parameter keywords now work for backwards compatibility.
x and y position are now added and the code definitely does work.
@rmhopkins4 rmhopkins4 closed this Aug 23, 2024
Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
Schelling small 🔵 +0.0% [-0.4%, +0.4%] 🔵 +1.6% [+1.2%, +1.9%]
Schelling large 🔵 +0.9% [+0.1%, +1.7%] 🔵 +1.5% [-1.5%, +5.6%]
WolfSheep small 🔵 +0.5% [-0.8%, +1.6%] 🔵 +0.6% [+0.3%, +0.9%]
WolfSheep large 🔵 +0.5% [-0.1%, +0.9%] 🔵 +0.5% [-0.7%, +1.6%]
BoidFlockers small 🔵 +0.9% [+0.4%, +1.4%] 🔵 +1.7% [+1.0%, +2.3%]
BoidFlockers large 🔵 +1.0% [+0.6%, +1.3%] 🔵 +1.5% [+0.8%, +2.1%]

@rmhopkins4 rmhopkins4 reopened this Aug 23, 2024
@EwoutH
Copy link
Member

EwoutH commented Aug 29, 2024

Thanks for this PR, sorry none of us got back to you (@projectmesa/maintainers we should be better in this - a full week is quite long).

It looks really useful. We indeed need to discuss how we handle variable names.

I might support going full matplotlib convention. So marker instead of shape, etc.

@rht
Copy link
Contributor

rht commented Aug 29, 2024

There is a problem with using Matplotlib nomenclature because Altair's API naming might be different. But it's better to pick either Matplotlib/Altair than to create a separate new naming scheme that nobody is familiar with. I haven't looked at Altair's but I assume the naming would be more modern.

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.

I really like the capabilities this brings. Thanks for working on this, good visualization is important.

This does need to be documented really well. Try if you can add some more examples and edge cases to your PR post.

solara.FigureMatplotlib(space_fig, format="png", dependencies=dependencies)


# used to make non(less?)-breaking change
# this *does* however block the matplotlib 'color' param which is somewhat distinct from 'c'.
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 explain or link to how color and c differ in matplotlib (scatter)?

cmap_exists = portray_data.pop("cmap", None)
norm_exists = portray_data.pop("norm", None)
vmin_exists = portray_data.pop("vmin", None)
vmax_exists = portray_data.pop("vmax", None)
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 feeling this can be done more elegant

@EwoutH
Copy link
Member

EwoutH commented Sep 20, 2024

@rmhopkins4 are you still interested in working on this? If so, how can I help?

@jackiekazil
Copy link
Member

jackiekazil commented Sep 23, 2024

@EwoutH, while in a perfect world, we hope that @rmhopkins4 hasn't moved on and will gloriously return to this awesomeness to carry the torch, but if they have - I wonder if we should create a tag to recognize incomplete but valuable PRs that might be picked up in the future for completion? (Maybe it takes 2 core contributors to agree on the potential -- so we don't add the tag to every PR.)

@EwoutH
Copy link
Member

EwoutH commented Sep 23, 2024

Yeah I agree this one is worth getting in. Maybe @Corvince or @rht wants to finish it?

For now we can just add it to the 3.0 milestone.

@EwoutH EwoutH added the enhancement Release notes label label Sep 23, 2024
@EwoutH EwoutH added this to the v3.0 milestone Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Release notes label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants