nkremerh / sugarscape

Development repository for the Digital Terraria Lab implementation of the Sugarscape agent-based societal simulation.
https://github.com/digital-terraria-lab/sugarscape
MIT License
7 stars 12 forks source link

Bentham decision model currently uses magic numbers for critical calculations #58

Closed nkremerh closed 4 months ago

nkremerh commented 5 months ago

The Bentham class in ethics.py relies upon magic numbers (carried over from the initial implementation) in some crucial lines (not a complete list):

23 # Timesteps to reach cell, currently 1 since agents only plan for the current timestep
24 timestepDistance = 1
...
32 # Agent discount, futureDuration, and futureIntensity implement Bentham's purity and fecundity
33 discount = 0.5
...
37 # Assuming agent can only see in four cardinal directions
38 extent = neighborhoodSize / (neighbor.vision * 4) if neighbor.vision > 0 else 1

It would be preferable to replace these magic numbers with information which can be interrogated from the individual agents. This information did not exist during the initial implementation but has since been included.

As a first pass, consider the inclusion of the following lines in ethics.py:

# Normalize future intensity by number of adjacent cells
cellNeighbors = len(neighbor.cell.neighbors)
futureIntensity = cellNeighborWealth / (globalMaxWealth * cellNeighbors)
# Normalize extent by total cells in range
cellsInRange = len(neighbor.cellsInRange) if neighbor.vision > 0 else 1
extent = neighborhoodSize / cellsInRange if neighbor.vision > 0 else 1
futureExtent = futureNeighborhoodSize / cellsInRange if neighbor.vision > 0 and self.lookahead != None else 1

These replace the usage of magic numbers and assumed constants with information directly pulled from agents. However, the results of test simulations deviate between the current, magic number based version and these new calculations dropped in. Theoretically, these should be the same if both versions are given the same default configuration (with only the seed value change, such as 12345).

Opening this up for thoughts as we enter the design phase of this update.

colinhanrahan commented 5 months ago

It looks like the deviation is solely due to the fact that cellsInRange can be restricted by both vision and movement, not only vision as is suggested by the old calculation. When agents have lower movement than vision, they shouldn't have vision * 4 cells in range; this is already included in the logic for findCellsInRange. It's an improvement to simulation accuracy, so I think it's a good change.

Some other thoughts on this code:

nkremerh commented 4 months ago

Resolved by #78.