Create abmtime function for StandardABM

[Tortar]

This is practically costless, and actually somewhat needed: run! and offline_run! restart from 0 in the step column each time they are called. Apart from that, this is a function some people would implement by themselves anyway to track the time of the ABM, and implementing it would make StandardABM more homogeneous with the new EventQueueABM which has one.

This means that this is a breaking change though if we change those two functions to instead use the time of the model

cc @Datseris

[Datseris]

yes i've been thinking about this as well, to add a timer in standardABM as well. Now run! and co should start from the model timer which itself starts at 0 but is never reset (unless re-creating the model of course).

[Tortar]

and so I guess also step! with a function should change:

Step the model forwards until f(model, t) returns true,
where t is the current amount of time the model has been evolved
for, starting from 0.

(the docstring is even not very clear in my opinion)

I would remove the t dependency and just do f(model), the user could use abmtime inside if wants to. Notice that this could be non breaking, since we can easily enough detect which one of the two signatures is called, it is just a deprecation

[Datseris]

Okay, but it has to be breaking. It is bad for performance to check if a user gave in a function expecting 1 or 2 arguments. Type unstable. (I know from DynamicalSystems.jl)

[Tortar]

actually it seems not bad with this trick:

julia> h(x) = x
h (generic function with 1 method)

julia> h(x, y) = x + y
h (generic function with 2 methods)

julia> t(f) = try; f(1); return true; catch; f(1,2); return false; end;

julia> using BenchmarkTools

julia> @btime t($h)
  1.182 ns (0 allocations: 0 bytes)
true

(same when h(x) is not defined)

[Datseris]

this is a missleading benchmark because you are passing constant literals to f: 1, 2. The compiler optimizes everything away and understands that f is called with 1, whose value is true. Everything is inlined away to the trivial t(f) = true.

[Datseris]

it is well known (stated in the manual in performance tips) that one should not use try blocks inside performance critical code, doubly more so not inside a for loop which is what step! does. Here the try block is simply compiled away!

[Tortar]

good observation, but I guess we are still in a similar situation since I think model is constant propagated, and I would use 0 for t, let's revise this in the future

[Tortar]

notice also that we would call it outside the for loop inside step!, at the start of the function, just a warning there is enough

[Tortar]

applicable is probably more robust though:

julia> f(x) = x
f (generic function with 1 method)

julia> t(f, x, y) = applicable(f, x, y) ? true : false
t (generic function with 1 method)

julia> using BenchmarkTools

julia> @btime t($f, $1, $1)
  93.312 ns (0 allocations: 0 bytes)
false

(the try catch trick takes microseconds when the catch block is executed instead...otherwise it is very fast though, so it could become a good reason for the user to update the code :D)

[Datseris]

or, we could simply break nothing. Make it so that the API is: f is user input, and f(model, t) returns true is when evolution stops. t is always the total amount of time elapsed since the start of the run! call. So, it does not care about the current model time.

Or, t is the same as the model time. Up to us to choose.

Since the model time is a new concept, either choice is non-breaking, given that we didn't explain this well enough originally. (What is breaking or not is defined by what the docs say)

[Tortar]

Or, t is the same as the model time. Up to us to choose.

In this case:

• it is redundant to pass it since it can be accessed with abmtime(model)
• (!) it actually changes the behaviour to use t as the model time, even if it is not stated in the docs, so this would be a silent breaking change, even if the docs weren't clear
• it is a bit more sensible to me f(model) since the stopping condition would probably be something unrelated to the time

---

On the other hand, leaving t as it is now it would be better in a sense since something like this is more easy to implement:

f(m, t) = stopping_condition_unrelated_to_time(m) || t > 1000

so in the end I think that keeping f(model, t) as it is now, and just improving the docs should be enough 👍

[Datseris]

it is redundant to pass it since it can be accessed with abmtime(model)

Sure, but it keeps the API the same, so it is non breaking in this case.

• even if it is not stated in the docs, so this would be a silent breaking change, even if the docs weren't clear

I don't think this is a breaking change. This is what I am trying to say: whether something is "breaking" or not is defined by the docs. No by what the actual code does. If something was ambiguous in the docs and could be interpreted either way, it does not count as a breaking change to enforce one of the ways it could be interpreted.

• since the stopping condition would probably be something unrelated to the time

I disagree! And that's the reason time was given as an argument in the first place. I would never use this functionality without an if t < 10000.0 or something condition, to avoid infinite while loops that never stop. But you already said the same thing in the second part of the answer.

[Tortar]

Yes, I'm more on the side to keep the current way since if you want to call twice step! with a function, it should be easier to implement a condition related to the number of steps if the time is relative to step!

On the breaking change part I don't agree but the subject is controversial :D : https://dev.to/turnerj/should-behavioural-changes-be-considered-breaking-changes-under-semver-2d5n

[Tortar]

So what is your opinion @Datseris on all this? I'm for keeping the time relative when a function is passed for the advantage I stated in the previous comment.

p.s. I can't help on Agents.jl for a while (some weeks) since I'm really short on time with uni work D:

[Datseris]

My opinion is the same. Keep the API as is and time is relative to model initial time.

No worries about the lack of time. I'll try to finish the event queue but also no promises.

[Tortar]

just a clarification since I'm not really sure that we are on the same bandwidth: by relative to the model time, do you mean that it starts from 0 each time you call the step! function just like it is already currently? because I think this is already the best between alternatives since if you call step! multiple times I think it is better this way Il Mar 5 Dic 2023, 10:29 George Datseris ***@***.***> ha scritto:

…

My opinion is the same. Keep the API as is and time is relative to model initial time. No worries about the lack of time. I'll try to finish the event queue but also no promises. — Reply to this email directly, view it on GitHub <#941 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQH6VXZEO4KYEWVMOR6GYXTYH3SPNAVCNFSM6AAAAABAD5P36KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBQGM3TAOBUHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[Datseris]

Yes, I mean that the counter inside step! always starts from 0 each time step is called. It measures time relative to model initial time.