-
Notifications
You must be signed in to change notification settings - Fork 9
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
KDr2
left a comment
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_tThis reverts commit 0355eab.
yebai
left a comment
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
AdvancedPSwe often need to mutate the input of the theTapedFunctionto handle random streams. It would be great if we could do something like:but for this to work we need to update the
args...andbindingsappropriately when wecopythe running task.This is just a draft, I'm not sure about all the implications of this change, especially with #138