Skip to content

Bugfix for non deterministic rng behavior #2598

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Jan 6, 2025
Merged

Conversation

quaquel
Copy link
Member

@quaquel quaquel commented Jan 6, 2025

This fixes a bug in how the numpy rng is seeded. If instantiating a model with Model(seed=42), rng should default to 42, not None. The net result of this bug is that at the moment, `Model(seed=42) will leave the numpy rgn undeterministic. This adds an explicit test to ensure that both model.random and model.rng behave in a deterministic manner when seeded with the same initial value and fixes the bug.

@quaquel quaquel added the bug Release notes label label Jan 6, 2025
@quaquel quaquel requested a review from EwoutH January 6, 2025 07:59
Copy link

github-actions bot commented Jan 6, 2025

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 -3.0% [-5.6%, -0.2%] 🔵 +1.0% [+0.8%, +1.1%]
BoltzmannWealth large 🔵 +1.2% [+0.6%, +1.7%] 🔵 +3.1% [+1.9%, +4.3%]
Schelling small 🔵 -0.2% [-0.5%, +0.2%] 🔵 +1.0% [+0.9%, +1.1%]
Schelling large 🔵 +0.7% [+0.3%, +1.0%] 🔵 -0.6% [-1.6%, +0.2%]
WolfSheep small 🔵 -0.5% [-1.1%, +0.2%] 🔴 +22.0% [+16.4%, +27.7%]
WolfSheep large 🔵 -2.0% [-2.7%, -1.5%] 🔵 -4.0% [-5.4%, -2.5%]
BoidFlockers small 🔵 +1.8% [+1.3%, +2.3%] 🔵 +0.2% [-0.5%, +0.9%]
BoidFlockers large 🔵 +3.3% [+2.3%, +4.3%] 🔵 +0.5% [+0.1%, +1.0%]

Copy link
Member

@EwoutH EwoutH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@quaquel quaquel merged commit 696923f into projectmesa:main Jan 6, 2025
11 checks passed
@quaquel quaquel deleted the rng branch January 6, 2025 09:09
jackiekazil pushed a commit that referenced this pull request Feb 14, 2025
* bugfix

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

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

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Release notes label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants