Skip to content
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

Deterministic seeded random streams across runs #194

Merged
merged 10 commits into from
Oct 15, 2024
16 changes: 12 additions & 4 deletions src/server.jl
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ end

# Implementation.

function remote_eval_fetch_channeled(worker, expr)
code = quote
put!(stable_execution_task_channel_in, $(QuoteNode(expr)))
take!(stable_execution_task_channel_out)
end
return Malt.remote_eval_fetch(worker, code)
end

function init!(file::File, options::Dict)
worker = file.worker
Malt.remote_eval_fetch(worker, worker_init(file, options))
Expand All @@ -106,7 +114,7 @@ function refresh!(file::File, options::Dict)
init!(file, options)
end
expr = :(refresh!($(options)))
Malt.remote_eval_fetch(file.worker, expr)
remote_eval_fetch_channeled(file.worker, expr)
end

"""
Expand Down Expand Up @@ -648,7 +656,7 @@ function evaluate_raw_cells!(
$(chunk.cell_options),
))

worker_results, expand_cell = Malt.remote_eval_fetch(f.worker, expr)
worker_results, expand_cell = remote_eval_fetch_channeled(f.worker, expr)

# When the result of the cell evaluation is a cell expansion
# then we insert the original cell contents before the expanded
Expand Down Expand Up @@ -827,7 +835,7 @@ function evaluate_raw_cells!(
# inline evaluation since you can't pass cell
# options and so `expand` will always be `false`.
worker_results, expand_cell =
Malt.remote_eval_fetch(f.worker, expr)
remote_eval_fetch_channeled(f.worker, expr)
expand_cell && error("inline code cells cannot be expanded")
remote = only(worker_results)
if !isnothing(remote.error)
Expand Down Expand Up @@ -888,7 +896,7 @@ function evaluate_params!(f, params::Dict{String})
:(@eval getfield(Main, :Notebook) const $(Symbol(key::String)) = $value)
end
expr = Expr(:block, exprs...)
Malt.remote_eval_fetch(f.worker, expr)
remote_eval_fetch_channeled(f.worker, expr)
return
end

Expand Down
13 changes: 13 additions & 0 deletions src/worker.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@ function worker_init(f::File, options::Dict)
project = WorkerSetup.LOADER_ENV[]
return lock(WorkerSetup.WORKER_SETUP_LOCK) do
return quote
# issue #192
# Malt itself uses a new task for each `remote_eval` and because of this, random number streams
# are not consistent across runs even if seeded, as each task introduces a new state for its
# task-local RNG. As a workaround, we feed all `remote_eval` requests through these channels, such
# that the task executing code is always the same.
const stable_execution_task_channel_out = Channel()
const stable_execution_task_channel_in = Channel() do chan
for expr in chan
result = Core.eval(Main, expr)

Choose a reason for hiding this comment

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

I think QuartoNotebookWorker has its own error handling, but you may want to catch errors from this call to prevent future put!(stable_execution_task_channel_in, ...) to fail after an exception.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm yes I might have to think about what to do in this case, but for normal notebook usage, barring bugs in QuartoNotebookRunner, the error handling inside the passed expressions should be sufficient (as indicated by the test suite passing). Of course it would be nicer if in case of bugs, the behavior was still somewhat robust

put!(stable_execution_task_channel_out, result)
end
end

push!(LOAD_PATH, $(project))

let QNW = task_local_storage(:QUARTO_NOTEBOOK_WORKER_OPTIONS, $(options)) do
Expand Down
17 changes: 17 additions & 0 deletions test/examples/random_seed.qmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
title: Random seed
---

```{julia}
using Random
Random.seed!(123)
rand()
```

```{julia}
rand()
```

```{julia}
rand()
```
37 changes: 37 additions & 0 deletions test/testsets/random_seed.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
include("../utilities/prelude.jl")

@testset "seeded random numbers are consistent across runs" begin
mktempdir() do dir
content = read(joinpath(@__DIR__, "../examples/random_seed.qmd"), String)
cd(dir) do
server = QuartoNotebookRunner.Server()
write("notebook.qmd", content)

jsons = map(1:2) do _
QuartoNotebookRunner.run!(server, "notebook.qmd"; showprogress = false)
end

function _output(cell)
try
only(cell.outputs).data["text/plain"]
catch e
@error "extracting outputs failed"
display(cell)
rethrow(e)
end
end

@test tryparse(Float64, _output(jsons[1].cells[2])) !== nothing
@test tryparse(Float64, _output(jsons[1].cells[4])) !== nothing
@test tryparse(Float64, _output(jsons[1].cells[6])) !== nothing

@test length(unique([_output(jsons[1].cells[i]) for i in [2, 4, 6]])) == 3

@test _output(jsons[1].cells[2]) == _output(jsons[2].cells[2])
@test _output(jsons[1].cells[4]) == _output(jsons[2].cells[4])
@test _output(jsons[1].cells[6]) == _output(jsons[2].cells[6])

close!(server)
end
end
end
Loading