Speedup of Agentset.shuffle #2010

quaquel · 2024-01-27T09:43:54Z

This speeds up shuffle by shuffling the actual weakrefs, rather than first turning them back into hard refs, shuffling them, and turning them back into weakness (which is what the old code did).

One probably controversial choice is that I have shifted to default inplace shuffle. Feel free to let me know what you think. I am fine either way but inplace is quite a bit faster (and the default behavior of random.shuffle).

for more information, see https://pre-commit.ci

commit c0de4a1 Author: Ewout ter Hoeven <E.M.terHoeven@student.tudelft.nl> Date: Sun Jan 21 14:43:08 2024 +0100 Add CI workflow for performance benchmarks (projectmesa#1983) This commit introduces a new GitHub Actions workflow for performance benchmarking. The workflow is triggered on `pull_request_target` events and can be triggered with a "/rerun-benchmarks" comment in the PR. It should be compatible with PRs from both forks and the main repository. It includes steps for setting up the environment, checking out the PR and main branches, installing dependencies, and running benchmarks on both branches. The results are then compared, encoded, and posted as a comment on the PR.

This reverts commit 9485087.

for more information, see https://pre-commit.ci

quaquel · 2024-01-27T09:47:08Z

How can I trigger the benchmark stuff?

github-actions · 2024-01-27T09:48:21Z

Performance benchmarks:

Model	Size	Init time [95% CI]	Run time [95% CI]
Schelling	small	🔵 +0.2% [-0.1%, +0.6%]	🟢 -11.1% [-11.2%, -11.0%]
Schelling	large	🔴 +93.7% [+46.5%, +141.0%]	🟢 -14.7% [-15.7%, -13.7%]
WolfSheep	small	🔵 +0.6% [+0.1%, +1.0%]	🟢 -18.1% [-18.2%, -18.0%]
WolfSheep	large	🔴 +32.7% [+4.0%, +65.2%]	🟢 -20.1% [-21.6%, -18.3%]
BoidFlockers	small	🔵 +1.0% [+0.4%, +1.6%]	🔵 +0.6% [-0.0%, +1.3%]
BoidFlockers	large	🔵 -0.2% [-1.0%, +0.7%]	🔵 +0.7% [+0.2%, +1.1%]

mesa/agent.py

quaquel · 2024-01-27T09:52:12Z

The benchmark init numbers make little sense to me. My changes only relate to shuffle and do, neither of which is touched by the model initialization.

EwoutH · 2024-01-27T09:54:32Z

Can you replicate them locally? Maybe with some more replications/seeds (modify benchmarks/configurations.py)?

quaquel · 2024-01-27T10:00:50Z

Can you replicate them locally? Maybe with some more replications/seeds (modify benchmarks/configurations.py)?

I'll try.

One possible solution, more generally, is to ensure the same seeds are used in both benchmarks. I'll look at the code and open a separate PR if needed.

EwoutH · 2024-01-27T10:05:37Z

One possible solution, more generally, is to ensure the same seeds are used in both benchmarks.

Should already be happening:

mesa/benchmarks/global_benchmark.py

Line 38 in 6a4ea54

for seed in range(1, config["seeds"] + 1):

So if "seeds" in the config say 25, it will use seed 1 to 25. It then does use the paired samples to compare it.

But if you see room for improvement, please do! Also check: #1983 (comment)

… agentset_speed

quaquel · 2024-01-27T10:58:37Z

Can you replicate them locally? Maybe with some more replications/seeds (modify benchmarks/configurations.py)?

I ran it locally with the default settings. The results are more in line with what I would expect. It seems the init is particularly noisy. Not really surprising but good to know.

Model	Size	Init time [95% CI]	Run time [95% CI]
Schelling	small	🔵 -0.2% [-0.7%, +0.2%]	🟢 -9.5% [-9.7%, -9.4%]
Schelling	large	🔵 -1.6% [-3.5%, +0.4%]	🟢 -25.0% [-29.1%, -20.9%]
WolfSheep	small	🔵 -3.8% [-4.6%, -2.9%]	🟢 -20.5% [-21.2%, -19.7%]
WolfSheep	large	🔵 -11.0% [-24.2%, -1.5%]	🟢 -24.4% [-28.0%, -20.7%]
BoidFlockers	small	🔵 -3.2% [-5.6%, -0.6%]	🔵 -2.9% [-4.1%, -1.9%]
BoidFlockers	large	🟢 -7.1% [-8.1%, -6.2%]	🟢 -5.0% [-6.8%, -3.2%]

quaquel · 2024-01-27T11:00:43Z

This is ready for review and merging. When merging, please squash and merge.

mesa/agent.py

Corvince

I am wondering if a similar logic could be applied to there other methods or in place of the current _update method. I would think similar performance improvements might be possible?

mesa/agent.py

quaquel · 2024-01-27T13:05:36Z

I am wondering if a similar logic could be applied to there other methods or in place of the current _update method. I would think similar performance improvements might be possible?

It's a bit tricky. Both select and sort typically need the actual agent object and thus cannot operate only on the weakrefs as is possible with shuffle.

EwoutH · 2024-01-27T13:48:57Z

This PR is starting to look ready to me. How good is the test coverage of this part of the code?

github-actions · 2024-01-27T13:54:07Z

Performance benchmarks:

Model	Size	Init time [95% CI]	Run time [95% CI]
Schelling	small	🔵 +0.6% [+0.1%, +1.1%]	🟢 -11.4% [-11.6%, -11.3%]
Schelling	large	🔴 +96.2% [+47.7%, +142.9%]	🟢 -14.2% [-15.7%, -12.4%]
WolfSheep	small	🔵 +1.1% [+0.6%, +1.7%]	🟢 -18.1% [-18.3%, -18.0%]
WolfSheep	large	🔵 +39.6% [+2.1%, +80.7%]	🟢 -13.1% [-15.0%, -11.2%]
BoidFlockers	small	🔵 +3.3% [+2.8%, +3.8%]	🔵 +0.7% [+0.0%, +1.4%]
BoidFlockers	large	🔵 +2.7% [+1.7%, +3.8%]	🔵 +1.0% [+0.4%, +1.6%]

EwoutH · 2024-01-27T13:57:10Z

Do we have any idea why this could slow down the shelling large init? It happened on both CI runs, with remarkable consistency.

quaquel · 2024-01-27T14:16:46Z

This PR is starting to look ready to me. How good is the test coverage of this part of the code?

We added testcode as part of #2007. This already covers inplace=True and inplace=False.

Do we have any idea why this could slow down the shelling large init? It happened on both CI runs, with remarkable consistency.

Yes, but not on my machine. Moreover, the init code is unrelated to this change. Shuffle is not called in model.__init__. Remember, the init is very short so tiny changes result in large percentages quickly.

rht · 2024-01-27T21:39:20Z

@quaquel protip: with

mesa/CONTRIBUTING.rst

Line 30 in 6a4ea54

    
           - Run ``git config pull.rebase true``. This prevents messy merge commits when updating your branch on top of Mesa main branch.

, when you do git pull, it will automatically be git pull --rebase.

rht · 2024-01-27T21:42:24Z

Squashed and merged with the commit message perf: Speed up Agentset.shuffle (#2010).

rht · 2024-01-27T23:08:14Z

mesa/agent.py

+        self.random.shuffle(weakrefs)
+
+        if inplace:
+            self._agents.data = {entry: None for entry in weakrefs}


How come in this line there is no need for (agent := ref()) is not None), but it is needed in L204?

When you create a new AgentSet, it has its own internal WeakkeyDict. Each WeakkeyDict creates its own weakrefs.

As can be seen in the docs, weakref.ref takes an optional callback. In the case of WeakkeyDict, this callback is a method on the dict itself (i.e., self.remove). So, when not shuffling in place, I have to unpack the weakrefs to ensure the new WeakkeyDict behaves correctly.

Moreover, AgentSet takes an iteratable of agents as an argument. Not an iterable of weakref. ref instances.

Hope this clarifies.

quaquel and others added 26 commits January 15, 2024 11:23

Make RandomActivationByType.agents_by_type backward compatible

bc1718c

[pre-commit.ci] auto fixes from pre-commit.com hooks

a7ddad5

for more information, see https://pre-commit.ci

updated warning

1f9af49

Merge remote-tracking branch 'upstream/main'

59885b6

Merge remote-tracking branch 'upstream/main'

b9fe806

Update .gitignore

2076b6d

Merge remote-tracking branch 'upstream/main'

600c372

Update global_benchmark.py

c83179d

pep8 rename

4c8cb8b

Update schelling.py

7bccdd1

Update schelling.py

64d8c46

Update schelling.py

1e794d3

Update schelling.py

4bc5fac

Revert "Squashed commit of the following:"

5cd0e74

This reverts commit 9485087.

Update .gitignore

63eec5e

further updates

2a4459c

Update benchmarks/WolfSheep/__init__.py

1b5661e

Merge remote-tracking branch 'upstream/main'

2be6424

Merge remote-tracking branch 'upstream/main'

c2e9313

Merge remote-tracking branch 'upstream/main'

7b8b7ea

shuffle update

dcf1e50

Merge remote-tracking branch 'upstream/main'

c40a324

Merge branch 'main' into agentset_speed

fefe684

switch to walrus operator in do

d196386

[pre-commit.ci] auto fixes from pre-commit.com hooks

4ef66de

for more information, see https://pre-commit.ci

EwoutH added Performance enhancement Release notes label labels Jan 27, 2024

EwoutH reviewed Jan 27, 2024

View reviewed changes

mesa/agent.py Outdated Show resolved Hide resolved

quaquel added 2 commits January 27, 2024 11:53

revert inplace to False

f910106

Merge branch 'agentset_speed' of https://github.com/quaquel/mesa into…

f7072ab

… agentset_speed

rht reviewed Jan 27, 2024

View reviewed changes

mesa/agent.py Show resolved Hide resolved

Corvince reviewed Jan 27, 2024

View reviewed changes

mesa/agent.py Outdated Show resolved Hide resolved

remove old code

9710914

EwoutH added trigger-benchmarks Special label that triggers the benchmarking CI and removed trigger-benchmarks Special label that triggers the benchmarking CI labels Jan 27, 2024

Corvince approved these changes Jan 27, 2024

View reviewed changes

rht merged commit 449d230 into projectmesa:main Jan 27, 2024
13 of 14 checks passed

rht reviewed Jan 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Speedup of Agentset.shuffle #2010

Speedup of Agentset.shuffle #2010

quaquel commented Jan 27, 2024 •

edited

Loading

quaquel commented Jan 27, 2024

github-actions bot commented Jan 27, 2024

quaquel commented Jan 27, 2024

EwoutH commented Jan 27, 2024

quaquel commented Jan 27, 2024

EwoutH commented Jan 27, 2024

quaquel commented Jan 27, 2024

quaquel commented Jan 27, 2024

Corvince left a comment

quaquel commented Jan 27, 2024

EwoutH commented Jan 27, 2024

github-actions bot commented Jan 27, 2024

EwoutH commented Jan 27, 2024

quaquel commented Jan 27, 2024 •

edited

Loading

rht commented Jan 27, 2024

rht commented Jan 27, 2024

rht Jan 27, 2024

quaquel Jan 28, 2024

Speedup of Agentset.shuffle #2010

Speedup of Agentset.shuffle #2010

Conversation

quaquel commented Jan 27, 2024 • edited Loading

quaquel commented Jan 27, 2024

github-actions bot commented Jan 27, 2024

quaquel commented Jan 27, 2024

EwoutH commented Jan 27, 2024

quaquel commented Jan 27, 2024

EwoutH commented Jan 27, 2024

quaquel commented Jan 27, 2024

quaquel commented Jan 27, 2024

Corvince left a comment

Choose a reason for hiding this comment

quaquel commented Jan 27, 2024

EwoutH commented Jan 27, 2024

github-actions bot commented Jan 27, 2024

EwoutH commented Jan 27, 2024

quaquel commented Jan 27, 2024 • edited Loading

rht commented Jan 27, 2024

rht commented Jan 27, 2024

rht Jan 27, 2024

Choose a reason for hiding this comment

quaquel Jan 28, 2024

Choose a reason for hiding this comment

quaquel commented Jan 27, 2024 •

edited

Loading

quaquel commented Jan 27, 2024 •

edited

Loading