DH Code Review #1
Conversation
repo setup
Risky food simulation
…hunt Preliminary Mesa code for stag hunt game
Document hawk/dove convergence logic + collect model status (running/convergence)
…totals Include agent totals per risk level in model data collection
…-riskadjust track when agents update risk level; implement convergence check based on stable risk attitudes
There was a problem hiding this comment.
I may add on later, depending on whether I see anything else in a second read-through, however I wanted to get this review posted ASAP since it contains the only major point of confusion I've found so far in a fairly thorough scan. I was not able to find any errors in the risk adjustment/distribution generation in HawkDoveMulti.
Overall, the software here looks excellent 🙂. The comments, tests, and general cleanliness were all well-appreciated while reading through this
| to the range of agent risk level.""" | ||
| ratio = self.model.max_risk_level / self.model.observed_neighborhood | ||
| # always round to an integer | ||
| return round(ratio * self.num_dove_neighbors) |
There was a problem hiding this comment.
This scaling is a bit confusing since it seems as though its making the # of dove neighbors proportional to a neighborhood size of max_risk_level = 9
Example case where it seems to be inconsistent, at least with my understanding:
With 6 (out of 8) dove neighbors this will return 7 at which point an agent with risk_level=7 will play HAWK, despite the actual number of neighbors being less than their risk aversion level
There was a problem hiding this comment.
I think the general strategy of scaling the # of doves to the range of risk levels which corresponds to the "base" neighborhood of 8 is perfectly fine, though it may benefit from being noted down in narrative docs/READMEs in order to explain how a choice is made when observed neighborhood is greater or less than 8
| return super().converged | ||
|
|
||
| return ( | ||
| self.schedule.steps > max(self.adjust_round_n, 50) |
There was a problem hiding this comment.
Will there ever be/what happens if the convergence condition of num_agents_risk_changed == 0 occurs before step 50?
At a glance, it seems like an arbitrary number so perhaps just a comment about its purpose if it is an intentional limit would help with clarity
There was a problem hiding this comment.
basically the model is supposed to run at least 50 steps? Does the system could show the dynamics of reaching convergence and then get out of it again , i.e. with the random_play_odds != 0.00
| "--max-steps", | ||
| help="Maximum steps to run simulations if they have not already " | ||
| + "converged (default: %(default)s)", | ||
| default=1000, # new convergence logic seems to converge around 400 |
There was a problem hiding this comment.
would there be any benefit to tunable convergence conditions via model parameter(s)? Might be more relevant for the single/no risk adjustment convergence logic which may be of less importance..
I'm not sure if there is anything further that can be done with the HawkDoveMulti convergence condition. Is it possible for there to be 'temporary stability' where num_agents_risk_changed == 0 for one risk adjustment and then some agents manage to adjust the next?
|
|
||
| # in variable risk with risk adjustment, numbers are not strictly equal | ||
| # but do get close and fairly stable; round to two digits before comparing | ||
| rounded_set = set([round(x, 2) for x in self.recent_rolling_percent_hawk]) |
There was a problem hiding this comment.
For my understanding: In the deck of recent phawk values, round all values to two digits, and compare if they are still different. If we have 15 values to compare and all of them are the same after rounding, this is considered converged.
There was a problem hiding this comment.
Hi , in general it looks fine. The overall design is a bit confusing to me.
The only thing I found is that apparently the play_neighborhood size can be larger then the grid size?
E.g. calling model = HawkDoveMultipleRiskModel(3, play_neighborhood=24) gives no error. Does that influence the modeling outcome?
| # agent with r = 0 should always take the risky choice | ||
| # (any risk is acceptable). | ||
| # agent with r = max should always take the safe option | ||
| # (no risk is acceptable) |
There was a problem hiding this comment.
this direction is confusing to me, maximal risk means r=0? so r is not the risk.
| # (any risk is acceptable). | ||
| # agent with r = max should always take the safe option | ||
| # (no risk is acceptable) | ||
| if self.proportional_num_dove_neighbors >= self.risk_level: |
There was a problem hiding this comment.
same here: if enough doves are in my neighborhood , i risk to play hawk.
|
|
||
|
|
||
| class HawkDoveAgent(mesa.Agent): | ||
| """ |
There was a problem hiding this comment.
General question on this agent: Design is a bit confusing. No step function necessary? How is the agent activated?
There was a problem hiding this comment.
a sorry got it, this is kind of the base class
| | H | 0, 0 | 3, 1| | ||
| | D |1, 3| 2, 2| | ||
|
|
||
| BACKGROUND: An unpublished paper by Simon Blessenohl shows that the equilibrium in this game is different for EU maximizers than for REU maximizers (all with the same risk-attitude), and that REU maximizers do better as a population (basically, play DOVE more often) |
There was a problem hiding this comment.
Please extend EU and REU, I do not understand the differences. A bit more background information to this distinction would be helpful.
| model adjust_neighborhood size""" | ||
| return self.get_neighbors(self.model.adjust_neighborhood) | ||
|
|
||
| @cached_property |
There was a problem hiding this comment.
Another novice question: why did you choose this pattern of cached_property and then getattr?
| } | ||
| for risk_level in range(self.min_risk_level, self.max_risk_level + 1): | ||
| field = f"total_r{risk_level}" | ||
| model_reporters[field] = field |
There was a problem hiding this comment.
I do not see how this counter works.
| return super().converged | ||
|
|
||
| return ( | ||
| self.schedule.steps > max(self.adjust_round_n, 50) |
There was a problem hiding this comment.
basically the model is supposed to run at least 50 steps? Does the system could show the dynamics of reaching convergence and then get out of it again , i.e. with the random_play_odds != 0.00
| assert all([len(agent.play_neighbors) == 4 for agent in model.schedule.agents]) | ||
|
|
||
| # neighborhood of 24 (grid needs to be at least 5x5) | ||
| model = HawkDoveSingleRiskModel(5, play_neighborhood=24, agent_risk_level=5) |
There was a problem hiding this comment.
I can run a model HawkDoveSingleRiskModel(3, play_neighborhood=24, agent_risk_level=3), and this does not trow an error. There seems to be a check from grid size to playable neighborhood missing? or not working?
I do not know how the grid neighborhood works for small grids with radius 2 neighbors. Are agents then just repeated?
|
Thank you so much @sgfost @maltevogl for your time reviewing this code and engaging with the research and modeling behind it! Your feedback is so valuable and I'm really grateful for the things you've caught and your suggestions and insights. I've implemented fixes for the two highest priority errors you flagged (invalid grid size & neighborhood size combinations, inaccurate proportional scaling of neighborhood sizes) and will be rerunning analysis and investigating convergence logic and behavior next week as I prepare to present on this work at DH2024 and update results for the articles we're working on. |
The ticket for this code review is DHCodeReview/DHCodeReview#6
I'm including code for two related Mesa models:
I'm primarily interested in feedback on the second one, but since it extends the simpler one I thought it would be useful to include that code.
I've included some unit tests and batch running code in the review in case it's useful for context but that's a lower priority.
The full repository is available at https://github.com/Princeton-CDH/simulating-risk
A running version of the application is available online. Here are links to view the two models that are under review, in case it's helpful to see the logic running:
https://simrisk.pulcloud.io/hawkdove-single
https://simrisk.pulcloud.io/hawkdove-multiple