-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add seed parameter #335
base: master
Are you sure you want to change the base?
Add seed parameter #335
Conversation
(I thought about this and forgot to fix it) |
There is a problem in that this only works if the benchmarks take the same number of samples. I don't know if this should be enforced or marked as a caveat: "The same seed guarantees the same sequence of random numbers, but the benchmarks may still pick an unequal number of random numbers". In practice, consistency increases even if it's not guaranteed to be perfect. |
Probably a dumb question, but why not give a seed to each benchmarkable instead? |
I guess one could. But the seed needs to be set before an entire benchmark run, not for each sample. And if it's not for a group you can just run An alternative is to add a hook to |
Given this recent Discourse thread, I think a seed for each benchmarkable would also make sense: What do you think @gustaphe ? |
I didn't think that would work, but it took me like a second to convince myself it would. If nobody's made a PR like that by Saturday I will |
Wait, isn't it already possible to set the seed in the setup code of each benchmarkable? |
No. If you set the seed in the julia> b = @benchmarkable sleep(rand([0, 0.5])) setup=(Random.seed!(1234))
Benchmark(evals=1, seconds=5.0, samples=10000)
julia> r = run(b)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
Range (min … max): 1.531 μs … 48.249 μs ┊ GC (min … max): 0.00% … 0.00%
Time (median): 1.739 μs ┊ GC (median): 0.00%
Time (mean ± σ): 1.836 μs ± 1.147 μs ┊ GC (mean ± σ): 0.00% ± 0.00%
▃▆██▅▄▃▁
▂▂▂▃▃▅▇█████████▇▆▅▄▄▃▃▃▃▃▃▃▃▂▃▂▃▃▂▃▂▂▂▂▂▂▂▂▂▂▂▁▁▁▂▂▁▂▁▁▁▂ ▃
1.53 μs Histogram: frequency by time 2.57 μs <
Memory estimate: 192 bytes, allocs estimate: 5. vs julia> b = @benchmarkable sleep(rand([0, 0.5])) setup=(Random.seed!(1235))
Benchmark(evals=1, seconds=5.0, samples=10000)
julia> r = run(b)
BenchmarkTools.Trial: 10 samples with 1 evaluation.
Range (min … max): 501.700 ms … 502.117 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 501.726 ms ┊ GC (median): 0.00%
Time (mean ± σ): 501.802 ms ± 156.542 μs ┊ GC (mean ± σ): 0.00% ± 0.00%
█ ▃
█▇▇▁▇▁▁▁▁▁▁▁█▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▇▁▁▁▁▁▁▇ ▁
502 ms Histogram: frequency by time 502 ms <
Memory estimate: 192 bytes, allocs estimate: 5. each of these runs only ever use a single result from |
I would guess that the most useful use of this parameter is supplying it as |
@@ -85,6 +85,7 @@ You can pass the following keyword arguments to `@benchmark`, `@benchmarkable`, | |||
- `gcsample`: If `true`, run `gc()` before each sample. Defaults to `BenchmarkTools.DEFAULT_PARAMETERS.gcsample = false`. | |||
- `time_tolerance`: The noise tolerance for the benchmark's time estimate, as a percentage. This is utilized after benchmark execution, when analyzing results. Defaults to `BenchmarkTools.DEFAULT_PARAMETERS.time_tolerance = 0.05`. | |||
- `memory_tolerance`: The noise tolerance for the benchmark's memory estimate, as a percentage. This is utilized after benchmark execution, when analyzing results. Defaults to `BenchmarkTools.DEFAULT_PARAMETERS.memory_tolerance = 0.01`. | |||
- `seed`: A seed number to which the global RNG is reset every benchmark run, if it is non-negative. This ensures that comparing two benchmarks gives actionable results, even if the running time depends on random numbers. Defaults to `BenchmarkTools.DEFAULT_PARAMETERS.seed = -1` (indicating no seed reset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this can have surprising side effects since it changes the global rng. It's a good thing that seed = -1
disables it by default, I'm just trying to imagine a way this can come back to bite us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Negative seeds will soon be allowed JuliaLang/julia#51416
It's better to use a different value, e.g. nothing
, as the "no seed given" sentinel.
I'm not clear on the details here, but instead of asking for a seed, why not copy the state of the global RNG at the beginning EDIT: this is what |
I'd go one step further and let the user provide the entire |
@gustaphe what do you think of this proposal? |
I've been waiting for a good time to read up on this/try it out but haven't found it. The obstacles I see are:
Like I said, I don't know if these are reasonable objections, I just haven't had time to think carefully about them. |
Serializing an e.g. |
Partial solution for #201 . Since it needs to be serializable, I opted for just a seed number.
seed!
is not super well documented, soInt
may not be the perfect type, but it's pretty handy. Negative seeds appear to be invalid, so I could use that as a stand-in fornothing
, asUnion{Int, Nothing}
ruined serialization.