Emphasize default scheduling behavior and consequences for adding Agents

[hbsmith]

Is your feature request related to a problem? Please describe. Problem: It's not obvious that the default behavior of Agents.jl is that when you add an Agent to your model, it may or may not be activated on the step that it's added.

The reason it's not predictable is because the default scheduler (fastest) returns an iterator, and if you add an agent and it happens to be added in the KeySet downstream of where you are in the for id in activation_order loop https://github.com/JuliaDynamics/Agents.jl/blob/7bd121435c025ed3cdb659d8d68dcba1a3a9eaf4/src/simulations/step.jl#L61, the agent will be activated on the same step. If it's upstream, it won't be activated until the next step. This can also have cascading effects where an agent adds an agent adds an agent... all on the same step.

This seems like "intended" behavior to some degree, since it's documented that schedules can be iterators, and the default scheduler happens to be an iterator. But it's far from obvious for someone building a model under mostly non-advanced conditions why they might sometimes experience new agents being activated on the step they're added, and sometimes not.

While the docstring for the fastest scheduler says that the agents are activated in an arbitrary sequence...

A scheduler that activates all agents once per step in the order dictated by the agent's container, which is arbitrary (the keys sequence of a dictionary). This is the fastest way to activate all agents once per step.

...it's not mentioned that not only is it arbitrary, but that is actually is an iterator that's updated in-place while stepping (a bad practice which is easy enough to avoid if you realize it's happening).

This is also not explicitly mentioned in the docs on scheduling, or on adding agents. I only discovered the "A note on iteration" (https://juliadynamics.github.io/Agents.jl/stable/api/#A-note-on-iteration-1) while diving through closed issues relevant to this issue. See e.g. https://github.com/JuliaDynamics/Agents.jl/issues/193, where the last commenthttps://github.com/JuliaDynamics/Agents.jl/issues/193#issuecomment-609061114 is not necessarily correct. And see: https://github.com/JuliaDynamics/Agents.jl/issues/457.

It seems like most (all?) the other built-in schedulers call collect at some point, so I think this is only relevant to fastest?

I think this is also only relevant to adding agents, as removing them shouldn't cause any issues since the iterator updates in-place.

Describe the solution you'd like At the very least, it would be good to emphasize/warn in the docstrings and docs that adding an Agent will cause unpredictable activation under the default scheduler of fastest, and to emphasize that the fastest returned not just an arbitrary ordering, but one that gets updated in-place during activation of agents during a model step.

A better solution would be to somehow make sure that the behavior for adding agents is both predictable and documented, even when the scheduler is an iterator. I don't have any elegant ideas of how to do the goal is to avoid collecting the agent ids when the scheduler is an iterator.

Describe alternatives you've considered Another alternative is to not default to a scheduler which is an iterator over the agents, especially when it's within the realm of reason that default users could be adding agents during steps.

[Tortar]

Ohh, didn't notice this behaviour, thanks for putting the time to investigate that!

A general idea would be changing the internals by some extent, but I don't think this is better than a simple collect, which anyway, correct me if I'm wrong, shouldn't be necessary with an UnremovableABM.

The changing would be adding agents to a vector which can be read by the scheduler so that before the next step the new agents are scheduled without the need to run collect on all the previously added agents. We manage removals by counting how many times remove_agent has been called and adjust the scheduler after some threshold. This seems a bit too complicated though, maybe it will slowdown everything and those operations have to be stopped for custom schedulers, which is messy. Another problematic aspect is that we should probably assume people uses step! to run the simulation, which I don't actually know if we already restricted them to do so.

[Datseris]

It is mentioned in the docs: https://juliadynamics.github.io/Agents.jl/stable/api/#A-note-on-iteration-1

The question is, how often do you think this should be mentioned? Because I would think that saying

A scheduler that activates all agents once per step in the order dictated by the agent's container, which is arbitrary (the keys sequence of a dictionary).

is already enough to highlight that the iterator is the key sequence of a dictionary and hence all "pitfalls" that apply to dynamic iterators apply here as well.

A better solution would be to somehow make sure that the behavior for adding agents is both predictable and documented, even when the scheduler is an iterator.

No. Not only there is no reason to do this, because once basic understanding is in place this becomes obsolete, but it also causes performance defecits.

A general idea would be changing the internals by some extent

No, because the iterator works exactly as intended so there is nothing to change.

Another alternative is to not default to a scheduler which is an iterator over the agents, especially when it's within the realm of reason that default users could be adding agents during steps.

No, because the dynamic schedulers are faster and I am very, very certain, that the overwhelming majority of users would want the default behavior to be the fastest one.

---

Overall, the important thing to keep is mind is a phrase I always use when giving workshops on writing documentation: weight every sentence in the docs with gold. And I do not think that this is a majorly undocumented issue to justify adding several new documentation all over the place, and does not call for changing any source code.

Where I find a point of agreement is to add only an extra sentence in the fastest scheduler docstring that says "this scheduler is dynamic, see "a note in iteration" in the documentation for limitations this may bring."