-
Notifications
You must be signed in to change notification settings - Fork 174
Update boltzmann example for mesa 3.1.5
#247
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
base: main
Are you sure you want to change the base?
Conversation
@Spartan-71 can you update tests please to pass. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 PR! There are some changes that I have mentioned in the review, but other than that it looks good to go:)
@@ -0,0 +1,20 @@ | |||
from mesa.experimental.cell_space import CellAgent |
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.
Could you change line this to "from mesa.discrete_space.cell_agent import CellAgent"?
cell_space has been moved out of experimental to the stable version, however, your version will continue to work as the changes were designed to be backward compatible. So just to avoid any ambiguity I think I'd prefer if it were changed
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.
cell_space has been moved out of experimental to the stable version
It doesn't look like it, based on the API references for the stable version.
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.
Check latest for the correct API. stable is the current release.
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.
Hey @sanika-n, I made the changes, but the pytests are failing for some reason.
The error messages are also not correct. Looks like something is going wrong.
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.
Oh, I will look into it later today
@@ -1,5 +1,8 @@ | |||
import mesa | |||
import networkx as nx | |||
from mesa.experimental.cell_space import Network |
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.
Same here, this should be replaced with "from mesa.discrete_space.network import Network"
|
||
## Further Reading | ||
|
||
The full tutorial describing how the model is built can be found at: | ||
https://mesa.readthedocs.io/en/latest/tutorials/intro_tutorial.html | ||
|
||
This model is drawn from econophysics and presents a statistical mechanics approach to wealth distribution. Some examples of further reading on the topic can be found at: | ||
|
||
[Milakovic, M. A Statistical Equilibrium Model of Wealth Distribution. February, 2001.](https://editorialexpress.com/cgi-bin/conference/download.cgi?db_name=SCE2001&paper_id=214) |
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.
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.
Yeah, even I'm getting the same error. Should I consider removing the same?
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.
Yes, go ahead
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.
Can we find another working link instead?
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.
@quaquel I searched for the same paper on different websites, but all of them are showing "NO DOWNLOADS AVAILABLE".
References:
https://econpapers.repec.org/paper/scescecf1/214.htm
https://ideas.repec.org/p/sce/scecf1/214.html
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.
Here is the link --- https://arxiv.org/abs/cond-mat/0211175
model_params = { | ||
"num_agents": Slider( | ||
label="Number of agents", | ||
value=10, | ||
min=5, | ||
max=20, | ||
step=1, | ||
), | ||
"num_nodes": Slider( | ||
label="Number of nodes", | ||
value=10, | ||
min=5, | ||
max=20, | ||
step=1, | ||
), | ||
} |
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 noticed that you handled the case where the number of agents is more than the number of nodes in the code (as one node only supports one agent) by ignoring the given value of number of nodes, which I think is a good approach. I would prefer that you let the user know that you are doing this though, by butting some sort of warning when the user does set the number of nodes<number of agents. I am saying this as the visualization will be counter intuitive if the user hasn't read the code. For instance, I set the number of nodes to 8 here but the visualization shows 18
I would prefer it if the warning were given in the GUI, but if that's too hard I think a terminal message should do it
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.
|
||
self.datacollector = mesa.DataCollector( | ||
model_reporters={"Gini": compute_gini}, | ||
agent_reporters={"Wealth": lambda _: _.wealth}, |
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 understand that agent_reporters is not used explicitly in the code, but do you think we should just leave it in if the user wants to do post-simulation analysis?
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.
Yes, I feel it should be the user's choice
3.1.4
3.1.5
@@ -1,6 +1,6 @@ | |||
import mesa | |||
import networkx as nx | |||
from mesa.experimental.cell_space import Network | |||
from mesa.discrete_space import Network |
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.
It should be "mesa.discrete_space.network import Network"
Changes:
model
andagent
class in different files.solara
support for network visualization.requirements.txt
Issue reference: #246