Deprecate seed parameter in favor of rng in Model#3147
Conversation
|
Performance benchmarks:
|
EwoutH
left a comment
There was a problem hiding this comment.
Thanks for this extensive effort, appreciated.
Overal looks good, a (small) note in the migration guide should be added and a few nitpicks.
mesa/model.py
Outdated
| self._seed = seed # this allows for reproducing stdlib.random | ||
| elif rng is None: | ||
| warnings.warn( | ||
| "the use of seed is deprecated, use rng instead", |
There was a problem hiding this comment.
There is nothing in the migration guide on this. Also, I would argue that the message is all you need. Just use rng instead of seed.
There was a problem hiding this comment.
While the actual code change is indeed very small, that change is made for a reason and has implications. Therefor I do think an entry is warranted.
Completeness is also important here. The migration guide should contain everything you need to know when updating to a new Mesa version.
It's also a chance to educate our users.
I think something like this would be a good start (feel free to edit of course).
## Mesa 3.5.0
### Deprecation of `seed` in favor of `rng`
The `seed` parameter on `Model.__init__` and the `reset_randomizer` method are deprecated in favor of [[SPEC-7](https://scientific-python.org/specs/spec-0007/)](https://scientific-python.org/specs/spec-0007/) compliant NumPy random number generation.
NumPy's `Generator` class provides higher-quality random number generation compared to Python's built-in `random` module or NumPy's legacy `RandomState`. The new `rng` parameter follows scientific Python community standards and offers better reproducibility controls.
```python
# Old
model = MyModel(seed=42)
model.reset_randomizer(seed=42)
# New
model = MyModel(rng=42)
model.reset_rng(rng=42)
```
Within your model, use `model.rng` (a `numpy.random.Generator`) instead of `model.random` for random number generation. The `rng` parameter accepts integers, sequences of integers, `numpy.random.SeedSequence`, `numpy.random.Generator`, or `numpy.random.BitGenerator`.
* Ref: [PR #2352](https://github.com/mesa/mesa/pull/2352)](https://github.com/mesa/mesa/pull/2352)There was a problem hiding this comment.
But most of what is said in your suggested text is not what this PR does. Model.random still exists. If you used seed and now shift to rng, the model will even behave the same. Really, the only thing that has changed is that we prefer using the rng keyword over the seed keyword, without any change in underlying logic or behavior.
There was a problem hiding this comment.
Ah right, I thought this was a bigger shift.
mesa/model.py
Outdated
| seed: A new seed for the RNG; if None, reset using the current seed | ||
| """ | ||
| warnings.warn( | ||
| "the use of seed is deprecated, use rng instead", |
Co-authored-by: Ewout ter Hoeven <[email protected]>
Co-authored-by: Ewout ter Hoeven <[email protected]>
|
For my context, is this the first step in a series of deprecations towards SPEC-7 compliance, of is this (for now) the only one? What's the future of I feel somehow we should communicate to our users why we're changing this and how that impacts them. |
This is all that is required to become compliant with spec-7. Spec-7 specifies the use of
To be decided. I prefer fully removing it inside Mesa, but this has profound performance consequences. I also figured out that
Like I said, the impact at the moment is only a change in preferred keyword (to become spec-7 compliant). We could explain this in the release notes. |
EwoutH
left a comment
There was a problem hiding this comment.
Thanks for the context.
Minor suggestion on the message, and if we include it in the release notes that should be enough.
seed on Model.__init__seed parameter in favor of rng in Model
Co-authored-by: Ewout ter Hoeven <[email protected]>
Co-authored-by: Ewout ter Hoeven <[email protected]>
This PR deprecates the
seedparameter inModel.__init__()andModel.reset_randomizer()in favor of the SPEC-7 compliantrngparameter. Users will now see aFutureWarningwhen usingseed, encouraging migration torng.Migration
This is a simple parameter name change with no behavioral differences:
The functionality remains identical - both approaches seed the random number generators in the same way. This change is part of aligning Mesa with SPEC-7 (scientific Python community standards for random number generation).
Notes
model.random(Python'srandom.Random) continues to exist and function as beforemodel.rng(NumPy'sGenerator) continues to exist and function as beforeseedorrngparameterRef: #2352 (original
rngimplementation)