Fix memory leak in ContinuousSpace agent removal#3031
Conversation
- Fix stale index entries in _index_to_agent after agent removal - Add test coverage for agent removal scenarios - Fixes mesa#3029
|
Performance benchmarks:
|
| old_index = self._agent_to_index[agent] | ||
| self._agent_to_index[agent] = old_index - 1 | ||
| self._index_to_agent[old_index - 1] = agent | ||
| last_old_index = old_index |
There was a problem hiding this comment.
By definition, last_old_index = self.active_agents[-1], so there is little need to update this inside the loop.
There was a problem hiding this comment.
since the last shifted agent is always at active_agents[-1], its old index is just len(active_agents) (after deletion). I can simplify this to avoid updating last_old_index on every iteration.
| if last_old_index is not None: | ||
| self._index_to_agent.pop(last_old_index, None) |
There was a problem hiding this comment.
this can only happen in the number of agents is 0, right?
There was a problem hiding this comment.
It happens when removing the last agent (nothing to shift), not specifically when count reaches 0. But since line 148 already deletes that index, the None check just prevents redundant cleanup.
|
The bug is even a bit more severe, at least theoretically. Since dictionaries are ordered in Python by insertion, the old index in edit: at present, |
|
@quaquel Good point about dict ordering! The fix prevents both the memory leak and potential wrong lookups if |
- Remove redundant per-iteration update of last_old_index - Calculate it directly as len(active_agents) after shifting - Clarify logic with improved comments - Addresses reviewer feedback on PR mesa#3029
|
Thanks! |
2 participants
Fixes #3029
Description
When removing an agent from ContinuousSpace, the internal index mapping (
_index_to_agent) was corrupted. The code shifted subsequent agents to fill the gap but failed to delete the old index entry of the last shifted agent. This created a "Ghost Agent" entry where two indices pointed to the same agent, causing the dictionary to grow larger than the number of active agents.Root Cause
In the _remove_agent method, when agents were shifted down to fill gaps, the old index entry of the last shifted agent was never deleted.
Example: Removing agent B from
[A, B, C]:_index_to_agent = {0: A, 1: B, 2: C}_index_to_agent = {0: A, 1: C, 2: C}(ghost entry at index 2)_index_to_agent = {0: A, 1: C}Changes
last_old_indexduring agent shiftingImpact