Fix MultiGrid._empty_mask not updated correctly#3019
Conversation
|
Performance benchmarks:
|
| if self._empties_built: | ||
| self._empties.discard(pos) | ||
| self._empty_mask[agent.pos] = True | ||
| self._empty_mask[pos] = False |
There was a problem hiding this comment.
Why did you change the boolean on the empty_mask both in place_agent and remove_agent?
mesa.space is legacy code and will be deprecated. I haven't implemented any of this, so I am trying to get my head around what the empty_maks is supposed to do.
There was a problem hiding this comment.
Just to add: I think the key issue is to clarify what true and false mean in this mask. The proper place to do so would be in line 823, where the mask is initialized. It seems true means that a cell is empty, so then the changes you have made make sense.
There was a problem hiding this comment.
@quaquel Thanks for reviewing!
_empty_mask tracks cell occupancy: True = empty, False = occupied.
The Bug: The boolean values were inverted, and updates were inside if self._empties_built:, so they only ran when that flag was True.
The Fix:
- place_agent(): Set
_empty_mask[pos] = False(occupied) - moved outside conditional - remove_agent(): Set
_empty_mask[pos] = True(empty) - moved outside conditional
This keeps _empty_mask synchronized with grid state, fixing select_cells(only_empty=True).
There was a problem hiding this comment.
I figured as much, but it's good to see it confirmed. Could you add a comment to line 823 explaining the meaning of 'true' and 'false' in the mask? Otherwise, this looks fine to me.
There was a problem hiding this comment.
@quaquel Done! I've added the comment at line 823 explaining that True = cell is empty, False = cell is occupied.
EwoutH
changed the title
2 participants
Fixes #3017
MultiGrid.place_agent()and remove_agent() were not correctly updating the _empty_mask array, causing select_cells(only_empty=True) to incorrectly return occupied cells.Changes
_empty_mask[pos] = False(occupied) outside the conditional block_empty_mask[pos] = True(empty) when cell becomes emptyRoot Cause
The _empty_mask update was placed inside the
if self._empties_built:block, so it only executed when that flag was True. Also, the boolean values were inverted compared to SingleGrid.