-
Notifications
You must be signed in to change notification settings - Fork 10
Update args at copy #141
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
Update args at copy #141
Conversation
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 think storing the args in TapedTask
will be more intuitive and it can keep the API unchanged.
diff --git a/src/tapedtask.jl b/src/tapedtask.jl
index 71656a3..8b10506 100644
--- a/src/tapedtask.jl
+++ b/src/tapedtask.jl
@@ -6,6 +6,7 @@ end
struct TapedTask{F}
task::Task
tf::TapedFunction{F}
+ args
produce_ch::Channel{Any}
consume_ch::Channel{Int}
produced_val::Vector{Any}
@@ -13,10 +14,11 @@ struct TapedTask{F}
function TapedTask(
t::Task,
tf::TapedFunction{F},
+ args,
produce_ch::Channel{Any},
consume_ch::Channel{Int}
) where {F}
- new{F}(t, tf, produce_ch, consume_ch, Any[])
+ new{F}(t, tf, args, produce_ch, consume_ch, Any[])
end
end
@@ -161,7 +163,7 @@ Base.IteratorEltype(::Type{<:TapedTask}) = Base.EltypeUnknown()
function Base.copy(t::TapedTask)
tf = copy(t.tf)
- new_t = TapedTask(tf)
+ args = do_tape_copy_on(t.args...) # !!!
+ new_t = TapedTask(tf, args...)
storage = t.task.storage::IdDict{Any,Any}
new_t.task.storage = copy(storage)
new_t.task.storage[:tapedtask] = new_t
This reverts commit 0355eab.
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.
Many thanks, @KDr2 and @FredericWantiez- it looks good and I'm happy to merge once CI passes.
In
AdvancedPS
we often need to mutate the input of the theTapedFunction
to handle random streams. It would be great if we could do something like:but for this to work we need to update the
args...
andbindings
appropriately when wecopy
the running task.This is just a draft, I'm not sure about all the implications of this change, especially with #138