Adding Boltzman Model and WolfSheep Model to Mesa_RL #197
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Updating Branch
There was a problem hiding this comment.
Good work @harshmahesheka I am looking forward to presenting this via George Mason. The big comment is please switch to the Solara visualization and there are some other questions and comments throughout.
| import os | ||
|
|
||
| import mesa | ||
| from mesa.visualization.ModularVisualization import ModularServer |
There was a problem hiding this comment.
Why are you using the older visualization set up instead of Solara Viz?
| MoneyModelRL, [grid, chart], "Money Model", {"N": 10, "width": 10, "height": 10} | ||
| ) | ||
| server.port = 8521 # The default | ||
| server.launch() |
There was a problem hiding this comment.
Please update to Solara visualization
| if len(cellmates) > 1: | ||
| # Choose a random agent from the cellmates | ||
| other_agent = random.choice(cellmates) | ||
| if other_agent.wealth > self.wealth: |
There was a problem hiding this comment.
This is more a nice to have, but this seems to make the result deterministic regardless of the RL, it seems you would get their eventually after enough steps, RL just makes it more efficient.
Is is possible/easy to make it so the agent gets to choose who it gives wealth to so it "learns" to give wealth to someone with less money?
There was a problem hiding this comment.
The idea was to keep these examples really simple and easy to train, hence only dimensional action space. But if you want I can change it
| """ | ||
| Create a new WolfRL-Sheep model with the given parameters. | ||
| """ | ||
| super().__init__( |
There was a problem hiding this comment.
Do you need to inherit all this from mesa_models? My concern is any change to that model will break this model and that inheriting these parameter is not necessary so it makes it unnecessarily brittle.
There was a problem hiding this comment.
I inherited it to show how to integrate your code with mesa easily. Not inheriting the code block and rewriting everything would weaken the whole point. Also, if we are changing something in the main example, we need to change it here as well to keep it updated. So, I think we can keep this arrangement, and whenever some major change takes place in the original examples, we verify here as well. The code is relatively simple, so it shouldn't be a major task.
|
|
||
|
|
||
| class SheepRL(Sheep): | ||
| def step(self): |
There was a problem hiding this comment.
Do you need to inherit this from mesa_models? My concern is any change to that model will break this model and that inheriting the Sheep class is not necessary so it makes it unnecessarily brittle.
| self.model.schedule.add(lamb) | ||
|
|
||
|
|
||
| class WolfRL(Wolf): |
There was a problem hiding this comment.
Do you need to inherit this from mesa_models? My concern is any change to that model will break this model and that inheriting these parameter is not necessary so it makes it unnecessarily brittle.
| import os | ||
|
|
||
| import mesa | ||
| import numpy as np |
There was a problem hiding this comment.
Please switch this to SolaraViz and not the old server
| "policy_wolf": PolicySpec(config=PPOConfig.overrides(framework_str="torch")), | ||
| }, | ||
| "policy_mapping_fn": lambda agent_id, *args, **kwargs: "policy_sheep" | ||
| if agent_id[0:5] == "sheep" |
There was a problem hiding this comment.
I am genuinely curious here, why is this only 0 to 5?
There was a problem hiding this comment.
Ah, this. Basically, agent_id is "Sheep[number]", and wolf is "Wolf[number]". So, we are checking the first 5 letters in Sheep to partition it. I will add a comment
Thanks for the appreciation. As discussed with @EwoutH and @rht, the visualization here is exactly the same as the one from mesa-examples. So, as soon as mesa-examples get updated. We can very easily update here. Basically as soon as we get this merged #154 |
|
The comments haven't been addressed. I'm fine with the PR being merged, because it is already useful in its current form. |
|
I am sorry, but I have been extremely busy with a research paper and my graduate school applications. I will address them within a week. We can leave it like this or merge and open an issue to address these. Whatever seems best to you all |
|
I have made most of the requested changes and rebutted my argument on which I haven't. We can go ahead and merge it from my side |
I have added the remaining two examples, the Boltzman Model and the WolfSheep model, to the rlo folder. The remaining thing would be to modify the main README.md to include a description of mesa_rl. Any suggestion on it is welcomed.
Currently, I have kept things similar to the previous pull request. After merging this, we can open an issue and discuss potential changes/improvements that were left behind.