MNT: more benchmark cleanup #26639
MNT: more benchmark cleanup #26639
Conversation
8d56a60 to
5e0dded
Compare
| @@ -13,7 +13,8 @@ class MeshGrid(Benchmark): | |||
| timeout = 10 | |||
|
|
|||
| def setup(self, size, ndims, ind, ndtype): | |||
| self.grid_dims = [(np.random.ranf(size)).astype(ndtype) for | |||
| rnd = np.random.RandomState(1864768776) | |||
There was a problem hiding this comment.
If we are already touching these, should we use the recommended
rng = np.random.default_rng(1864768776)
rng.random(size, dtype)
There was a problem hiding this comment.
My reading of NEP 19 is that we're supposed to use np.random.RandomState for stable, reproducible RNG sequences across numpy verisons, which is why I used it explicitly in this PR. I guess it's been quite a while since NEP 19 - is that no longer the best recommendation?
There was a problem hiding this comment.
@ngoldbaum you're correct here. The testing use case (which applies to benchmarking too) for RandomState hasn't changed.
|
RandomState has the advantage that the bit stream is guaranteed to stay the same, we don't make that promise for the new generators, so I think it is still the right choice for testing. Hmm, I wonder if that stability guarantee has implications for the threading work? |
|
Hmm. Not this PR, but maybe we should document the use-cases explicitly:
|
| array_class = array_type[0] | ||
| self.arr = getattr(SortGenerator, array_class)(self.ARRAY_SIZE, dtype, *array_type[1:]) | ||
| self.arr = getattr(SortGenerator, array_class)(self.ARRAY_SIZE, dtype, *array_type[1:], rnd) |
There was a problem hiding this comment.
The linter is annoyed at this line
There was a problem hiding this comment.
All LGTM, let's get this in. Thanks @ngoldbaum & reviewers.
Linter complaint is irrelevant, so ignoring. @mattip's suggestion to document the RandomState use case better in a follow-up sounds like a good idea to me.
In the vein of #26638, this makes more of the benchmarks reproducible. It also deletes some dead code.