fixes weakref bug in shuffe_do

[quaquel]

if one agent removes another agent, it could still be activated by shuffle_do. This adds a unit test to spot the bug and contains the fix.

longer explanation

shuffle_do used agents = list(self._agents.keys()). This returns a list of hardrefs to all agents. So if agents[0] removes agents[1], it will be removed from the model, but it is still activated in shuffle_do. we had the same bug before in shuffle in january.

[github-actions]

Performance benchmarks:

Model	Size	Init time [95% CI]	Run time [95% CI]
BoltzmannWealth	small	🔵 +1.0% [-0.0%, +2.1%]	🔵 +0.3% [+0.2%, +0.5%]
BoltzmannWealth	large	🔵 +0.4% [+0.0%, +0.9%]	🔵 +3.5% [+2.5%, +4.4%]
Schelling	small	🔵 +0.5% [+0.1%, +0.9%]	🔵 +0.1% [-0.1%, +0.4%]
Schelling	large	🔵 -0.1% [-1.1%, +0.9%]	🔵 +0.4% [-0.7%, +1.7%]
WolfSheep	small	🔵 +0.2% [-0.2%, +0.5%]	🔵 +1.9% [+1.6%, +2.1%]
WolfSheep	large	🔵 +1.8% [+0.6%, +3.0%]	🔴 +9.0% [+6.9%, +11.1%]
BoidFlockers	small	🔵 +0.7% [+0.3%, +1.1%]	🔵 -0.1% [-0.7%, +0.6%]
BoidFlockers	large	🔵 +0.1% [-0.5%, +0.7%]	🔵 -0.0% [-0.8%, +0.8%]

[EwoutH]

Right, this happens because an Agent can get removed within a step.

Have you done any benchmarks by any chance?

Edit: I still have some benchmark code here.

[quaquel]

Right, this happens because an Agent can get removed within a step.

Yes exactly. I ran into it with a wolf sheep version where I simply used self.agents.shuffle_do, rather than activate by class. The test now capture the problem.

Have you done any benchmarks by any chance?

Not beyond the default mesa benchmarks. Feel free to rerun your original tests. My hunch is that the performance after the fix will be marginally slower, but not by much than before the bugfix. The delta with chaining shuffle().do()should however still persist because you iterate over the agents only once instead of twice.

[EwoutH]

I’m going to approve, slow is better than buggy. Performance can be checked and optimized later.

Thanks!