Skip to content
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

Bug with timed event when no other events scheduled #54

Open
hdavid16 opened this issue Apr 29, 2023 · 3 comments
Open

Bug with timed event when no other events scheduled #54

hdavid16 opened this issue Apr 29, 2023 · 3 comments
Labels
bug Something isn't working
Milestone

Comments

@hdavid16
Copy link
Member

hdavid16 commented Apr 29, 2023

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 hdavid16 added the bug Something isn't working label Apr 29, 2023
@hdavid16
Copy link
Member Author

hdavid16 commented Apr 30, 2023

It seems that the issue is at:

function _run!(c::Clock, Δt::Float64)
c.end_time = c.time + Δt
_setTimes(c)
while any(i->(c.time i < c.end_time), (c.tn, c.tev))
_step!(c) == 0 || break

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...

@filchristou
Copy link
Contributor

filchristou commented Sep 20, 2023

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.

@hdavid16
Copy link
Member Author

A PR would be great. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants