Copy args

[FredericWantiez]

Seems like #141 has a small issue with copying the args around. From the last update:

function f(x)
  for i in 1:3
    produce(i + x)
  end
end

ttask = TapedTask(f, 1)
consume(ttask) # 2

ttask2 = copy(ttask)
consume(ttask2) # 3

ttask3 = copy(ttask; args=(5,))
consume(ttask3) # 3 (!!) x is still 1

but this runs fine

function f(x)
  for i in 1:3
    produce(i + x)
  end
end

ttask = TapedTask(f, 1)

ttask2 = copy(ttask)
consume(ttask2) # 2

ttask3 = copy(ttask; args=(5,))
consume(ttask3) # 6 correct update

[KDr2]

I think it makes no sense when one copies a started task and tries to re-initialize its arguments.

We can give an error when one tries to do this.

[yebai]

@FredericWantiez I agree with @KDr2 that this is a strange use case. Is any reason why we want to do it?

We can give an error when one tries to do this.

It sounds good to throw an error on this.

[FredericWantiez]

That would happen when we split a particle halfway through the program simulation. If we define the function like:

function (model::Model) (rng::AbstractRNG)
   ...
end

when we fork/copy the particle after it started running we need to change the reference to the rng to change its seed for example or split it.

[KDr2]

You can do it by overloading Libtask.tape_copy:

Libtask.tape_copy(rng::AbstractRNG) = a_new_rng

# or if this overload is too wide, you can make another type:
struct XRNG
  rng::AbstractRNG
  some_other_states
end

function (model::Model) (rng::XRNG)
   ...
end

Libtask.tape_copy(rng::XRNG) = a_new_xrng

This is the way we manipulate data while a fork is occurring.

[FredericWantiez]

Nice ! but in this case the reference in the TapedTask and the TapedFunction are different since we call tape_copy twice. First in copy(::TapedFunction) and a second time in tape_copy.(t.args):

Libtask.jl/src/tapedtask.jl

Lines 167 to 172 in 1cecf7d

	tf = copy(t.tf)
	task_args = if length(args) > 0
	typeof(args) == typeof(t.args) || error("bad arguments")
	args
	else
	tape_copy.(t.args)

[KDr2]

tape_copy.(t.args) is only done when we copy an unstarted task, and at this point I think args in the original task/tf is not filled into the bindings yet? So it's only tape_copied once, if not, it's bug.

[FredericWantiez]

We don't check that the task is running or not, as long as args is empty we would run tape_copy.(t.args). Even if we restrict to something like tf.counter <= 1 we don't pass the new bindings from copy(t.tf) to the new TapedTask.

That's an issue for something like:

Libtask.tape_copy(x::XRNG) = deepcopy(XRNG)

This is very similar to what we do in AdvancedPS to update the rng streams during the sampling procedure.

[KDr2]

We don't check that the task is running or not, as long as args is empty we would run tape_copy.(t.args).

In the latest code, we do the check, if the task is running and new args are provided, we throw an error.

About the issue you mentioned, I didn't get it, could you please provide an example to describe it? Thank you.

[FredericWantiez]

In the latest code, we do the check, if the task is running and new args are provided, we throw an error.

That's true but if we just copy without any args, we end up calling tape_copy twice:

ttask = Libtask.TapedTask(func, XRNG)
consume(ttask)

ttask2 = copy(ttask) # Here, we call both copy(t.tf) and tape_copy.(t.args)

For the reference part, the issue I'm facing in AdvancedPS is we often have something that looks like this:

mutable struct XRNG
  rng::AbstractRNG
end

tape_copy(rng::XRNG) = deepcopy(rng)

function (model::Model) (rng::XRNG)
  ...
end

rng1 = XRNG()
particle = TapedTask(model, rng1)
consume(particle)

sub_part1 = copy(particle)
sub_part2 = copy(particle)

Random.seed!(sub_part1.args.rng, 1) # Simplifying here, we should do it in tape_copy
Random.seed!(sub_part2.args.rng, 2) # Otherwise, they're both generating the same numbers

consume(sub_part1)
consume(sub_part2)

but the problem here is that the XRNG in sub_part1.args is not the one being forwarded in the bindings when we call model()

[KDr2]

Oh, you are right, could you try this fix #144 to see if it works well?