Fix reference of schedule #79
Conversation
src/ScheduledDiagnostics.jl
Outdated
| @@ -158,7 +158,7 @@ function ScheduledDiagnostic(; | |||
| reduction_time_func = nothing, | |||
| compute_schedule_func = EveryStepSchedule(), | |||
| output_schedule_func = isnothing(reduction_time_func) ? | |||
| compute_schedule_func : EveryStepSchedule(), | |||
| deepcopy(compute_schedule_func) : EveryStepSchedule(), | |||
There was a problem hiding this comment.
Why would this need a deep copy?
There was a problem hiding this comment.
Because in general compute_schedule_func contains a mutable object that is used to track when the it was last called
There was a problem hiding this comment.
Doesn't that mean that the deep copy can become out of sync from what is passed in (if the input has any mutable data)?
There was a problem hiding this comment.
Yes, that's exactly the problem being fixed here.
Suppose you have a struct that holds the last time the callback was called: if you use this struct for both compute and output, calling compute will update the last time the callback was called, so that output will never happen.
With a deepcopy, we allow them to be out of sync, so that they can happen independently.
Let me also note this only changes the default constructor that is currently never used.
Schedules have to be unique objects because they store information about they were last called. The constructor of `ScheduledDiagnostics` was using a reference instead of a copy, which led to incorrect scheduling.
Schedules have to be unique objects because they store information about they were last called. The constructor of
ScheduledDiagnosticswas using a reference instead of a copy, which led to incorrect scheduling.