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

Initialize Solara-based adv_tutorial #1726

Merged
merged 10 commits into from
Jul 13, 2023

Conversation

rht
Copy link
Contributor

@rht rht commented Jun 29, 2023

No description provided.

@rht rht force-pushed the adv_tutorial_experimental branch from 58e0993 to 4e75b8f Compare June 29, 2023 11:39
@codecov
Copy link

codecov bot commented Jun 29, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (65c5e6c) 79.96% compared to head (8e5057f) 79.96%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    projectmesa/mesa#1726   +/-   ##
=======================================
  Coverage   79.96%   79.96%           
=======================================
  Files          18       18           
  Lines        1178     1178           
  Branches      220      220           
=======================================
  Hits          942      942           
  Misses        202      202           
  Partials       34       34           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rht
Copy link
Contributor Author

rht commented Jun 29, 2023

You can try at https://colab.research.google.com/github/rht/mesa/blob/adv_tutorial_experimental/docs/tutorials/adv_tutorial_experimental.ipynb.

Currently, only the step button works fine. The play button doesn't properly update the grid and the timeseries.

tpike3

This comment was marked as outdated.

Copy link
Member

@tpike3 tpike3 left a comment

Choose a reason for hiding this comment

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

Sorry was tracking this as ready to go and then saw the comment about the buttons.

It is looking good
-- same with the buttons step works, play, worked but jumped states and seems to render slow and be intermittent.

@rht rht force-pushed the adv_tutorial_experimental branch 3 times, most recently from ac65075 to 35e7513 Compare July 4, 2023 13:11
@rht
Copy link
Contributor Author

rht commented Jul 4, 2023

The Colab issue was fixed by projectmesa/mesa-examples#47. I have just added a tutorial to write your custom visualization object via Matplotlib. It's a histogram, just like the current advanced tutorial. It's only 6 lines of code (excluding comments). Compare this with ~35 LOC of JS (client-side) and ~15 LOC of Python (server-side).

@rht rht force-pushed the adv_tutorial_experimental branch 3 times, most recently from 3dbc774 to d0bf217 Compare July 4, 2023 13:28
@rht
Copy link
Contributor Author

rht commented Jul 4, 2023

Compare this with ~35 LOC of JS (client-side) and ~15 LOC of Python (server-side).

Of the current advanced tutorial.

@rht rht force-pushed the adv_tutorial_experimental branch 5 times, most recently from bd5d24c to fe5381d Compare July 4, 2023 13:48
@rht
Copy link
Contributor Author

rht commented Jul 4, 2023

I have moved the experimental tutorial to be hidden in the "more" part of the menu bar. This is not discoverable at all. One option is to have the experimental tutorial be the default, and the old version to be linked from it instead.

@rht rht force-pushed the adv_tutorial_experimental branch from fe5381d to 9f7dc5d Compare July 8, 2023 12:53
@rht rht changed the title Initialize experimental adv_tutorial Initialize Solara-based adv_tutorial Jul 8, 2023
@rht
Copy link
Contributor Author

rht commented Jul 8, 2023

I have updated so that the Solara-based advanced tutorial is the one that people will see first in the menu bar.

One concern is that people will have to install from the Git repo of mesa-examples in order to install mesa-models. Should we move the Solara viz code to core Mesa? Though it's not a huge problem because it's just an extra pip install git+https... away.

@rht rht force-pushed the adv_tutorial_experimental branch from 9f7dc5d to 2f22f13 Compare July 8, 2023 12:58
@rht rht force-pushed the adv_tutorial_experimental branch from 2f22f13 to 24bdf62 Compare July 8, 2023 13:10
@rht
Copy link
Contributor Author

rht commented Jul 8, 2023

Any objections with moving the Solara viz to core Mesa, @jackiekazil @Corvince ?

@Corvince
Copy link
Contributor

Corvince commented Jul 8, 2023

Any objections with moving the Solara viz to core Mesa, @jackiekazil @Corvince ?

No objections from my side, I think solara is the perfect fit for mesa. That said both solara and the current implementation of the JupyterViz class seems quite rough around there edges. So it should definitely stay under experimental, like currently done in mesa_examples. And thus communicate that it is not currently part of our public API and not subject to semver, since I think there will be quite some changes in the near future.

The alternative to bringing this to mesa would again be a separate mesa_viz package. I don't know how big a dependency is in this regard.

@rht
Copy link
Contributor Author

rht commented Jul 8, 2023

At this point, the many benefits of having it in core Mesa outweighs the fact that it may still be volatile. It is mainly to expose the viz to many people early on (and that it is convenient to install from pip install mesa and no more), so that we have feedbacks sooner. As it is currently, we will only get feedbacks from new users who happen to read the visualization tutorial, not existing users.

@tpike3 is going to do an intro tutorial session at CSSSA. The choice is whether to have the tutorial code does pip install mesa, or it does pip install mesa and pip install mesa-models.

An example feedback/feature request would be to add figure title to the chart output (#954), and the PR #955. It seems to be hard to debug.

@Corvince
Copy link
Contributor

Corvince commented Jul 8, 2023

Yes i mean the idea has always been to have a mesa meta package. So pip install mesa would get you mesa-core, mesa-examples and mesa-viz. And only an advanced Installation procedure could only install mesa-core or any other mix. So I think from a (standard) user perspective it would be even simpler.

But that would require some substantial restructuring so I agree nothing for the foreseeable future. But we could later move from mesa.experimental to mesa-viz. If we put this directly into mesa.visualization I am worried it will lead to some false sense of stability.

@rht
Copy link
Contributor Author

rht commented Jul 9, 2023

Then I will add mesa-models as requirements of mesa. This means users can automatically import Boltzmann wealth model for quick demo. Depends on projectmesa/mesa-examples#52 to be merged.

@tpike3
Copy link
Member

tpike3 commented Jul 9, 2023

Then I will add mesa-models as requirements of mesa. This means users can automatically import Boltzmann wealth model for quick demo. Depends on projectmesa/mesa-examples#52 to be merged.

Done

@rht rht force-pushed the adv_tutorial_experimental branch from 64e1449 to 0d18bb3 Compare July 9, 2023 15:56
@jackiekazil
Copy link
Member

This is intended to go into 2.0 yes?
Are there any other solara tickets for 2.0?

@tpike3
Copy link
Member

tpike3 commented Jul 9, 2023

This is intended to go into 2.0 yes? Are there any other solara tickets for 2.0?

This should go with 2.0 and this is the only one. However, @rht the build is failing it looks like Solara's the problem, with its use of pydantic as the problem

It seems pydantic 2 had a breaking change

What do you think?

@rht
Copy link
Contributor Author

rht commented Jul 9, 2023

I have raised the issue on Solara Discord. Still waiting for their reply.

@rht rht force-pushed the adv_tutorial_experimental branch 2 times, most recently from fbc499d to 6734647 Compare July 12, 2023 11:08
@rht
Copy link
Contributor Author

rht commented Jul 12, 2023

The Solara Pydantic issue has been fixed in widgetti/solara#189, but there is another problem: mesa-models depends on mesa, and since I added mesa-models as a dependency of mesa, it is a circular dependency. As there is no mesa-core yet, I will have to remove mesa from mesa-models dependency. To save time, I will push directly the change to mesa-examples without a PR process.

@rht rht force-pushed the adv_tutorial_experimental branch from 937157a to af25509 Compare July 12, 2023 11:28
@rht rht force-pushed the adv_tutorial_experimental branch from c08139b to e7df413 Compare July 12, 2023 11:38
@rht
Copy link
Contributor Author

rht commented Jul 12, 2023

@tpike3 this is ready to merge.

@rht
Copy link
Contributor Author

rht commented Jul 12, 2023

Don't forget to update the v2.0 branch or this PR is not incorporated into the release.

@jackiekazil
Copy link
Member

@rht V2.0 branch wasn't be updated. When I move to release,I am going to cut a new v2.0 branch from what is in main... for easy access.

@jackiekazil
Copy link
Member

I am going to merge this. Looks good to me. Thank you @rht for your work on this.

@jackiekazil jackiekazil merged commit 950adb8 into projectmesa:main Jul 13, 2023
@rht rht deleted the adv_tutorial_experimental branch July 13, 2023 02:43
@jackiekazil jackiekazil added this to the Mesa 2.0 (Wellton) milestone Jul 13, 2023
@rht rht mentioned this pull request Jul 17, 2023
6 tasks
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