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

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

Closed
nkremerh opened this issue May 21, 2024 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@nkremerh
Copy link
Owner

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.

@nkremerh nkremerh added enhancement New feature or request help wanted Extra attention is needed labels May 21, 2024
@colinhanrahan
Copy link
Contributor

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:

  • len(neighbor.cell.neighbors) will always be the same for every neighbor (4 or 8 depending on neighborhood mode) so it can be calculated outside of the loop to reduce runtime. We saw previously that any len calls inside the loop drastically increased runtime, and this somewhat extends to other function calls too.
  • len(neighbor.cellsInRange) is going to be much more expensive. Maybe I can write a function to replace this if needed, or we could store this number when calculating cellsInRange.
  • I still don't think extent or futureExtent translate well from the felicific calculus and correlate with overall success/happiness in the way we think they do. It's supposed to represent something like "how many agents are affected by the pleasure", but the only agents affected during a move are the agent moving, the agent to be killed (if one exists), and agents who could have benefited more by going to that cell. I'm implementing this in my opportunity cost-based model which should be finished within a few days of starting work (off hours). You could also include debtors and calculate the potential benefit of bringing a child into the world—the latter might fix *Top models.

@nkremerh
Copy link
Owner Author

Resolved by #78.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants