DiscreteEvents.jl icon indicating copy to clipboard operation
DiscreteEvents.jl copied to clipboard

Bug with timed event when no other events scheduled

Open hdavid16 opened this issue 1 year ago • 3 comments

The following should not execute the scheduled event until t=10, but it gets executed when @run! is called:


julia> using DiscreteEvents

julia> c=Clock()
Clock 1: state=:idle, t=0.0, Δt=0.01, prc:0
  scheduled ev:0, cev:0, sampl:0


julia> f(c)=println(tau(c))
f (generic function with 1 method)

julia> @event f(c) at 10

julia> @run! c 1
10.0
"run! finished with 1 clock events, 0 sample steps, simulation time: 1.0"

However, if other events are scheduled before t=10, the event doesn't trigger when running up to t=1.

hdavid16 avatar Apr 29 '23 17:04 hdavid16

It seems that the issue is at:

https://github.com/JuliaDynamics/DiscreteEvents.jl/blob/4a9fab45a9af19c7d940a48714afb707b2e7598d/src/clock.jl#L291-L295

In the example above, c.time = 0.0 and c.end_time = 1.0. Since there is no sampling, c.tn = 0, which triggers the while loop. Triggering the while loop results in running _step!(c) which steps to the next event in the schedule. This corner case occurs because we are at c.time = 0. The logic needs to be fixed here so that _step!(c) is not triggered.

Proposed change is to first check if c.tn or c.tev is not zero and then check if it is between c.time and c.end_time:

    while any(i-> !iszero(i) && (c.time ≤ i < c.end_time), (c.tn, c.tev))
        _step!(c) == 0 || break
        if c.state == Halted()
            return c.end_time
        end
    end

However, this won't work if a simulation starts at a negative time point (instead of 0)...

@pbayer, I can create a PR for this if you think this is a good fix or we can discuss something more robust.

Perhaps, setting c.tn = nothing in _setTimes(c) when there is no temporal sampling would avoid issues with simulations that start at a negative time...

hdavid16 avatar Apr 30 '23 02:04 hdavid16

In a local branch, I substituted the while condition

while any(i->(c.time ≤ i < c.end_time), (c.tn, c.tev))

with the following

    while c.time ≤ c.tev < c.end_time || (c.time <= c.tn < c.end_time && c.tn != 0) || 
(c.time <= c.tn < c.end_time && _sampling(c))

It's pretty verbose but I think it works and at least it passes all tests. Actually it isolates the case where c.tn == 0 and when there is some other sort of sampling taking place.

If you like I can make PR from my branch. I also added a couple of related tests.

filchristou avatar Sep 20 '23 16:09 filchristou

A PR would be great. Thanks.

hdavid16 avatar Sep 20 '23 23:09 hdavid16