From d5c03f2967076dfe4edfc16c255a4c6efc8491c6 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Oct 2024 14:17:42 +0200 Subject: [PATCH 01/10] Create failing test --- test/examples/random_seed.qmd | 17 +++++++++++++++++ test/testsets/random_seed.jl | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 test/examples/random_seed.qmd create mode 100644 test/testsets/random_seed.jl diff --git a/test/examples/random_seed.qmd b/test/examples/random_seed.qmd new file mode 100644 index 0000000..e97b76c --- /dev/null +++ b/test/examples/random_seed.qmd @@ -0,0 +1,17 @@ +--- +title: Random seed +--- + +```{julia} +using Random +Random.seed!(123) +rand() +``` + +```{julia} +rand() +``` + +```{julia} +rand() +``` diff --git a/test/testsets/random_seed.jl b/test/testsets/random_seed.jl new file mode 100644 index 0000000..beef5ca --- /dev/null +++ b/test/testsets/random_seed.jl @@ -0,0 +1,34 @@ +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) + ipynb = "notebook.ipynb" + outputfiles = ["first.ipynb", "second.ipynb"] + jsons = map(outputfiles) do output + QuartoNotebookRunner.run!( + server, + "notebook.qmd"; + output, + showprogress = false, + ) + JSON3.read(output) + end + + _output(cell) = only(cell.outputs).data["text/plain"] + + @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 _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 From c12ff3bb71b5b82edb087ac8061a4b03701dca29 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Oct 2024 14:23:06 +0200 Subject: [PATCH 02/10] simplify --- test/testsets/random_seed.jl | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/testsets/random_seed.jl b/test/testsets/random_seed.jl index beef5ca..d46f7d9 100644 --- a/test/testsets/random_seed.jl +++ b/test/testsets/random_seed.jl @@ -6,16 +6,13 @@ include("../utilities/prelude.jl") cd(dir) do server = QuartoNotebookRunner.Server() write("notebook.qmd", content) - ipynb = "notebook.ipynb" - outputfiles = ["first.ipynb", "second.ipynb"] - jsons = map(outputfiles) do output + + jsons = map(1:2) do _ QuartoNotebookRunner.run!( server, "notebook.qmd"; - output, showprogress = false, ) - JSON3.read(output) end _output(cell) = only(cell.outputs).data["text/plain"] @@ -24,6 +21,8 @@ include("../utilities/prelude.jl") @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]) From 8b3ea0aeda61a664061f774a7219bd54ac7cc165 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Oct 2024 14:36:22 +0200 Subject: [PATCH 03/10] feed all eval requests through a single channel --- src/server.jl | 16 ++++++++++++---- src/worker.jl | 13 +++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/server.jl b/src/server.jl index f881d75..355fe61 100644 --- a/src/server.jl +++ b/src/server.jl @@ -92,6 +92,14 @@ end # Implementation. +function remote_eval_fetch_channeled(worker, expr) + code = quote + put!(var"__stable_execution_task_channel_in", $(QuoteNode(expr))) + take!(var"__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)) @@ -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 """ @@ -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 @@ -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) @@ -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 diff --git a/src/worker.jl b/src/worker.jl index 986b2c0..7d41c41 100644 --- a/src/worker.jl +++ b/src/worker.jl @@ -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 consistens across runs even if seeded, as each task introduces a new state for its + # task-local RNG. As a workaround, we use feed all `remote_eval` requests through these channels, such + # that the task executing code is always the same. + var"__stable_execution_task_channel_out" = Channel() + var"__stable_execution_task_channel_in" = Channel() do chan + for expr in chan + result = Core.eval(Main, expr) + put!(var"__stable_execution_task_channel_out", result) + end + end + push!(LOAD_PATH, $(project)) let QNW = task_local_storage(:QUARTO_NOTEBOOK_WORKER_OPTIONS, $(options)) do From 212724cb885a4d4b3f4e289cbe37379f883cd41a Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Oct 2024 14:37:06 +0200 Subject: [PATCH 04/10] formatting --- test/testsets/random_seed.jl | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/testsets/random_seed.jl b/test/testsets/random_seed.jl index d46f7d9..f55432b 100644 --- a/test/testsets/random_seed.jl +++ b/test/testsets/random_seed.jl @@ -8,11 +8,7 @@ include("../utilities/prelude.jl") write("notebook.qmd", content) jsons = map(1:2) do _ - QuartoNotebookRunner.run!( - server, - "notebook.qmd"; - showprogress = false, - ) + QuartoNotebookRunner.run!(server, "notebook.qmd"; showprogress = false) end _output(cell) = only(cell.outputs).data["text/plain"] From 7336afea93797eb9632a3abe5d53b2ac88fcd3db Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Oct 2024 14:42:49 +0200 Subject: [PATCH 05/10] `var"` macros not necessary after all --- src/server.jl | 4 ++-- src/worker.jl | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/server.jl b/src/server.jl index 355fe61..f1abab6 100644 --- a/src/server.jl +++ b/src/server.jl @@ -94,8 +94,8 @@ end function remote_eval_fetch_channeled(worker, expr) code = quote - put!(var"__stable_execution_task_channel_in", $(QuoteNode(expr))) - take!(var"__stable_execution_task_channel_out") + put!(stable_execution_task_channel_in, $(QuoteNode(expr))) + take!(stable_execution_task_channel_out) end return Malt.remote_eval_fetch(worker, code) end diff --git a/src/worker.jl b/src/worker.jl index 7d41c41..f90f546 100644 --- a/src/worker.jl +++ b/src/worker.jl @@ -7,11 +7,11 @@ function worker_init(f::File, options::Dict) # are not consistens across runs even if seeded, as each task introduces a new state for its # task-local RNG. As a workaround, we use feed all `remote_eval` requests through these channels, such # that the task executing code is always the same. - var"__stable_execution_task_channel_out" = Channel() - var"__stable_execution_task_channel_in" = Channel() do chan + 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) - put!(var"__stable_execution_task_channel_out", result) + put!(stable_execution_task_channel_out, result) end end From ce3d2651b68683534a5cf321af8cf70c6f2a4766 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Oct 2024 14:43:23 +0200 Subject: [PATCH 06/10] typos --- src/worker.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/worker.jl b/src/worker.jl index f90f546..68dc44d 100644 --- a/src/worker.jl +++ b/src/worker.jl @@ -4,8 +4,8 @@ function worker_init(f::File, options::Dict) return quote # issue #192 # Malt itself uses a new task for each `remote_eval` and because of this, random number streams - # are not consistens across runs even if seeded, as each task introduces a new state for its - # task-local RNG. As a workaround, we use feed all `remote_eval` requests through these channels, such + # 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 From 24a7effb0fd3a8998e5c6f379cf9d4467f26a4df Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Oct 2024 15:33:25 +0200 Subject: [PATCH 07/10] catch errors not seen locally to understand CI failures better --- test/testsets/random_seed.jl | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/testsets/random_seed.jl b/test/testsets/random_seed.jl index f55432b..9a9f4db 100644 --- a/test/testsets/random_seed.jl +++ b/test/testsets/random_seed.jl @@ -11,7 +11,15 @@ include("../utilities/prelude.jl") QuartoNotebookRunner.run!(server, "notebook.qmd"; showprogress = false) end - _output(cell) = only(cell.outputs).data["text/plain"] + 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 From 102fe6ddefcdc5732a3e1b395b3f7c2153d48cd9 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Oct 2024 16:03:08 +0200 Subject: [PATCH 08/10] add env to make `Random` available --- test/examples/random_seed/Project.toml | 2 ++ test/examples/{ => random_seed}/random_seed.qmd | 0 test/testsets/random_seed.jl | 12 ++---------- 3 files changed, 4 insertions(+), 10 deletions(-) create mode 100644 test/examples/random_seed/Project.toml rename test/examples/{ => random_seed}/random_seed.qmd (100%) diff --git a/test/examples/random_seed/Project.toml b/test/examples/random_seed/Project.toml new file mode 100644 index 0000000..576ba3c --- /dev/null +++ b/test/examples/random_seed/Project.toml @@ -0,0 +1,2 @@ +[deps] +Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" diff --git a/test/examples/random_seed.qmd b/test/examples/random_seed/random_seed.qmd similarity index 100% rename from test/examples/random_seed.qmd rename to test/examples/random_seed/random_seed.qmd diff --git a/test/testsets/random_seed.jl b/test/testsets/random_seed.jl index 9a9f4db..91e45bd 100644 --- a/test/testsets/random_seed.jl +++ b/test/testsets/random_seed.jl @@ -2,7 +2,7 @@ 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) + content = read(joinpath(@__DIR__, "../examples/random_seed/random_seed.qmd"), String) cd(dir) do server = QuartoNotebookRunner.Server() write("notebook.qmd", content) @@ -11,15 +11,7 @@ include("../utilities/prelude.jl") 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 + _output(cell) = only(cell.outputs).data["text/plain"] @test tryparse(Float64, _output(jsons[1].cells[2])) !== nothing @test tryparse(Float64, _output(jsons[1].cells[4])) !== nothing From 393b3e302cf21fff3b937d73ed07921505b192f1 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Oct 2024 16:03:37 +0200 Subject: [PATCH 09/10] formatting --- test/testsets/random_seed.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/testsets/random_seed.jl b/test/testsets/random_seed.jl index 91e45bd..0f264e1 100644 --- a/test/testsets/random_seed.jl +++ b/test/testsets/random_seed.jl @@ -2,7 +2,8 @@ include("../utilities/prelude.jl") @testset "seeded random numbers are consistent across runs" begin mktempdir() do dir - content = read(joinpath(@__DIR__, "../examples/random_seed/random_seed.qmd"), String) + content = + read(joinpath(@__DIR__, "../examples/random_seed/random_seed.qmd"), String) cd(dir) do server = QuartoNotebookRunner.Server() write("notebook.qmd", content) From ae3a3008d900bcf55c13ef32ccd749165ce919b6 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Oct 2024 16:34:39 +0200 Subject: [PATCH 10/10] remove copypasted code that unnecessarily created a new notebook file away from the project --- test/testsets/random_seed.jl | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/test/testsets/random_seed.jl b/test/testsets/random_seed.jl index 0f264e1..01853a4 100644 --- a/test/testsets/random_seed.jl +++ b/test/testsets/random_seed.jl @@ -1,30 +1,25 @@ include("../utilities/prelude.jl") @testset "seeded random numbers are consistent across runs" begin - mktempdir() do dir - content = - read(joinpath(@__DIR__, "../examples/random_seed/random_seed.qmd"), String) - cd(dir) do - server = QuartoNotebookRunner.Server() - write("notebook.qmd", content) + notebook = joinpath(@__DIR__, "../examples/random_seed/random_seed.qmd") - jsons = map(1:2) do _ - QuartoNotebookRunner.run!(server, "notebook.qmd"; showprogress = false) - end + server = QuartoNotebookRunner.Server() - _output(cell) = only(cell.outputs).data["text/plain"] + jsons = map(1:2) do _ + QuartoNotebookRunner.run!(server, notebook; showprogress = false) + 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 + _output(cell) = only(cell.outputs).data["text/plain"] - @test length(unique([_output(jsons[1].cells[i]) for i in [2, 4, 6]])) == 3 + @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 _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]) + @test length(unique([_output(jsons[1].cells[i]) for i in [2, 4, 6]])) == 3 - close!(server) - end - end + @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