Skip to content

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

Closed
rmhopkins4 wants to merge 10 commits intomesa:mainfrom
rmhopkins4:matplotlib-viz-param-refactor
Closed

More parameter functionality in matplotlib (default) drawer visualization#2242
rmhopkins4 wants to merge 10 commits intomesa:mainfrom
rmhopkins4:matplotlib-viz-param-refactor

Conversation

@rmhopkins4
Copy link
Copy Markdown
Contributor

@rmhopkins4 rmhopkins4 commented Aug 23, 2024 •
edited by EwoutH
Loading

(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

@rmhopkins4
Update matplotlib.py
8629310
@rmhopkins4
matplotlib visualization supports more params
2cc3f10 
No longer are shape, color, and marker explicitly implemented.
Instead, the implementation is more parameter-independent.
@rmhopkins4
old param keywords, x,y pos
bc67c60 
old parameter keywords now work for backwards compatibility.
x and y position are now added and the code definitely does work.
@rmhopkins4
implemented 'norm', reformatted colormap application
9632600
@rmhopkins4
formatting
ed6cf81
@rmhopkins4
update to cmap, norm flow
5b67ade
@rmhopkins4
colormap application moved to its own function
2e4cdd0
@rmhopkins4
Update matplotlib.py
bf59cbc
@rmhopkins4 rmhopkins4 closed this Aug 23, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 23, 2024

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
Copy Markdown
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
Copy Markdown
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.

EwoutH
EwoutH reviewed Aug 29, 2024
View reviewed changes
Copy link
Copy Markdown
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.

mesa/visualization/components/matplotlib.py


# used to make non(less?)-breaking change
# this *does* however block the matplotlib 'color' param which is somewhat distinct from 'c'.
Copy link
Copy Markdown
Member

@EwoutH EwoutH Aug 29, 2024

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)?

mesa/visualization/components/matplotlib.py
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
Copy Markdown
Member

@EwoutH EwoutH Aug 29, 2024

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 EwoutH mentioned this pull request Sep 1, 2024
Closed
@EwoutH
Copy link
Copy Markdown
Member

EwoutH commented Sep 20, 2024

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

@jackiekazil
Copy link
Copy Markdown
Member

jackiekazil commented Sep 23, 2024 •
edited
Loading

@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
Copy Markdown
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
@EwoutH EwoutH removed this from the v3.0 milestone Oct 2, 2024
@EwoutH
Copy link
Copy Markdown
Member

EwoutH commented Oct 4, 2024

As soon as #2336 is merged I'm going to try to revive this.

@EwoutH EwoutH mentioned this pull request Oct 4, 2024
Closed
@quaquel quaquel mentioned this pull request Oct 28, 2024
Merged
@quaquel
Copy link
Copy Markdown
Member

quaquel commented Nov 8, 2024

superseded by #2467

@quaquel quaquel closed this Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@EwoutH EwoutH EwoutH left review comments

Assignees

No one assigned

Labels

enhancement Release notes label

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rmhopkins4 @EwoutH @rht @jackiekazil @quaquel