Deprecate `agent_step!`/`model_step!` separation in favour of `step!`

Question

Deprecate `agent_step!`/`model_step!` separation in favour of `step!`

Closed this issue 8 months ago · 27 comments

After skim reading through this PR #889 and the related issue #883, I would like to comment on a somewhat related topic to this:

In the past I've previously proposed dropping agent_step! altogether and encourage users to only use model_step! instead. IIRC that was right before we switched from version 3 to 4. It was turned down back then due to not wanting to change the UX too much and with the argument that agent_step! provides a little bit of convenience for those users who create very simple models with Agents.jl and only need activation of agents once in each step.

Now it seems as if the user interface will drastically change anyways, so I'd like to bring this up again. (I hope I understood the reasoning behind it correctly, so bear with me please.) Given that the goal of this PR and the connected discussion behind it is that Agents.jl should get closer to the DynamicalSystems.jl ecosystem in terms of usage, it might make sense to finally drop the differentiation between those functions now and move towards a simple step! function interface just like it's used in the other packages.

The convenience argument still stands, of course, but I think that differentiating between agent_step! and model_step! obfuscates a bit what's happening in the background. Most importantly that would be the agent scheduling which is handled automatically with the given scheduler from the model struct. Explaining scheduling to users is not that complicated (it's all just a bit of [filtered] iteration anyways) but it would (i) enable them to move from simple to complicated models with less friction, and (ii) allow us to simplify the stepping code because we don't need to differentiate between different types of stepping functions and their desired order anymore.

Happy to hear your (preferably endorsing) thoughts on this whole idea. :)

Answer 1 · 2023-09-25T17:32:08.000Z

Given that the goal of this PR and the connected discussion behind it is that Agents.jl should get closer to the DynamicalSystems.jl ecosystem in terms of usage, it might make sense to finally drop the differentiation between those functions now and move towards a simple step! function interface just like it's used in the other packages.

You are correct in the goal assessment, but the point of making things more similar is to include the "dynamic rule" in the model structure, which is what happens in other ecosystems as well. Limiting the "dynamic rule" to be a single function brings similarity only more to DynamicalSystems.jl, as in DiffEq you have one rule for the equation, and another rule for the callback, and another rule for other things.

Another large reason for this change was to, in the future, allow the user to provide/configure different functions for the actions of different agent types, in hopes of optimizing the multi-agent models. In this case, limiting to one function would be impossible. Similarly, I plan to write a draft Gillespie model, where the structure of the dynamic rule is not clear yet. It is not clear whether it would be a single function dynamic_rule!(model). It is likely that it wont be like that.

So all in all enforcing that the dynamic rule must be one function with 1 input argument (the model) is not good for future plans that motivated the change in the first place.

On the other hand, I understand your argument that

but I think that differentiating between agent_step! and model_step! obfuscates a bit what's happening in the background

but, I could counter argue that having the scheduler as part of the model (as it is now) makes things more clear. In your counter proposal, where does the scheduler come from? Does it become a model property (part of the properties field?). Or we still have the scheduler as a field, and we instruct users to call the schedule(model) function, which uses the internal scheduler? I guess the latter is the best suggestion.

I am curious about:

move from simple to complicated models with less friction

Is it really true that real world research applications of ABM do not follow the simple rule of "step each agent once and if need be also step the model"? How many examples of this can you provide? If I am wrong and the majority of real world applications do not follow this simple system then sure, case closed, we don't need it.

In any case, I do support the idea. It will be more complex to teach how to create the model stepping function, but the package as a whole will be less complex due to the reduction in functions that go into SingleContainerABM. (Notice that I stress SingleContainerABM because other new types of models may have different ways to configure the dynamic rule)

The good thing is, it is spectacularly simple to deprecate this change in a non-breaking manner. We anyways have to deprecate step!(model, agent_step, model_step, ...) So if agent_step is not dummystep, we throw a warning saying that one function is used now.

I am worried though. This is a really large change that affects truly 100% of users. Is it wise to do it? On the other hand, you may say that including the dynamic rule also affacts 100% of the users. But at least in this case the users only change 2 lines of code: giving the dynamic rule to the model creation instead of the run! function. While in the case of merging agent and model steps, the users all will have to alter their model stepping functions. Is it really wise to put this much annoyance to potentially 8,000 users?

Answer 2 · 2023-09-25T17:41:02.000Z

I think this is an interesting idea.

I always found the obfuscation of the scheduling a bit strange, this doesn't happen in other softwares e.g. in Mesa. It seems to me that you are right that the scheduling isn't that complicated to explain to users and giving to them the freedom to experiment with that from the start allows for less friction.

I do support the idea too, given that we will actually just change this in a non-breaking way, even if I understand the concerns of @Datseris

Answer 3 · 2023-09-26T09:47:11.000Z

Thanks for your thoughts. :)

Is it really true that real world research applications of ABM do not follow the simple rule of "step each agent once and if need be also step the model"?

I can only speak from personal experience. In our models we have opted for solely using model_step! from the very beginning because our agents are not activated linearly (i.e. one after another) once per step. Instead we activate different groups of agents dynamically, sometimes even multiple times at each step. (For historic reference, see #331 and #351 from Fall 2020.)

in hopes of optimizing the multi-agent models

I'm personally all in favour of this since our newest model has 5 distinct agent types which make absolutely no sense to unify in one type. Right now the performance isn't abysmal, mind you, but a bit of speed up would be very much appreciated.

However, I haven't tracked the progress and discussion on the whole optimisation efforts for multi-agent models closely, so I don't really know your current ideas for it. If you think the proposed unification of agent_step! and model_step! would lead to a state in which multi-agent ABM optimisations are completely impossible, then that's a really really good argument against it.

Still, I think the problem would then persist for those people like me who cannot use the linear nature of agent_step! in their research. So I would naturally prefer a solution to the performance problem that doesn't rely on its existence but works more generally.

The good thing is, it is spectacularly simple to deprecate this change in a non-breaking manner. We anyways have to deprecate step!(model, agent_step, model_step, ...) So if agent_step is not dummystep, we throw a warning saying that one function is used now.

given that we will actually just change this in a non-breaking way

Maybe I misunderstand the concept of what's breaking and what's non-breaking. As far as I understood it, non-breaking implies that we can just throw a deprecation warning and then handle the merging of the stepping functions in the background. Surely we could just handle the use of agent_step! like we did before (execute it according to the model scheduler before/after the model_step! function) but then there wouldn't be any changes in our code as we would have to keep all the handling of the stepping functions.

If that's correct, I think it's impossible to make the change "strictly non-breaking" because we wouldn't want to handle the deprecation of agent_step! automatically but will require the user to merge their stepping functions into one step! function (i.e. what has been model_step! before). At least to me, it doesn't seem like a lot of effort to ask our users to put their model at the beginning/end of the unified step! function.

While in the case of merging agent and model steps, the users all will have to alter their model stepping functions.

Those users updating their models to work with Agents 6.0 will have their models as work-in-progress anyways. So incorporating their current draft of agent_step! into the unified step! will be a quick fix for them. They wouldn't even have to delete anything but just add three lines:

Before:

agent_step!(agent, model) = # agents do fancy things

function model_step!(m)
    # model does fancy things
end

After:

agent_step!(agent, model) = # agents do fancy things

function model_step!(m)
    for id in schedule(model)
        agent_step!(model[id], model)
    end
    # model does fancy things
end

obfuscation of the scheduling [...] doesn't happen in other softwares e.g. in Mesa

Agreed. To add to this, I always have NetLogo in mind because that's what almost everyone uses for teaching ABMs and also what a lot of people use for their actual research. There you have a simple go procedure which is more or less the unified step! function proposed here.

This is a really large change that affects truly 100% of users.

Definitely not 100% because I'm also a user (together with others in my team) and wouldn't be affected by that change at all. ;)

Is it wise to do it?

I don't know. But imho it's the right thing to do. :D

Answer 4 · 2023-09-26T10:14:24.000Z

Definitely not 100% because I'm also a user (together with others in my team) and wouldn't be affected by that change at all. ;)

Yes you would. Literally every single person using Agents.jl would. Right now you need to explicitly pass an agent stepping function irrespectively of if you use it or not. You are passing dummystep, but you are still passing something.

It is a big deal to change the API (input number of arguments) of the most used function of the package (step!/run!). It is important that we do not take this lightly at all.

That's why in the ongoing #889 we should make this as seemless and easy as possible. And make sure that code of v5 would work in v6. (besides the other breaking change with SVectors)

@Tortar do you think you can do this change? Not only that the functions are components of the model, but that you also deprecate step! so that when agent_step! is not dummystep there is an appropriate warning shown? I will co-contribute in your PR to update the Tutorial file.

Answer 5 · 2023-09-26T10:17:25.000Z

I also should stress that the argument "Other frameworks do it X way" is not an argument favouring X way.

Answer 6 · 2023-09-26T11:11:17.000Z

I also should stress that the argument "Other frameworks do it X way" is not an argument favouring X way.

Well it depends, users of those frameworks who happen to use Agents.jl will find it more familiar this type of behaviour. And also, if many took this decision about handling schedulers there will be a reason for it (but it obviously need concrete proofs anyway).

Answer 7 · 2023-09-26T11:29:27.000Z

I'm actually a bit concerned about the removing of agents, and the subsequent need to check if the id is in allids the user would need to do by himself. Isn't this something the framework should do instead of the user? I'm also asking myself if the current methodology with the agent stepping functions inside of the model is maybe enough because it is actually possible to keep everythin together, this is how:

both dummystep in struct for agent_step and model_step -> old version deprecated
both given in struct -> new version proposed by @Datseris
only model_step given in struct-> new version proposed by @fbanning

The third version will be considered advanced feature. Is it ok like this? We could make sure that in the third version a model_step is actually passed, but it's simple because model_step accept 1 parameters while agent_step accept 2.

(Not to be too pedantic about what other frameworks do, but this is at a closer inspection what Mesa does, but in a different way because of the OOP nature of Python)

Answer 8 · 2023-09-26T11:41:51.000Z

But you can say that is a bit unnecessary, because in the end it is just enough that the user passes dummystep for the agent_step in the version proposed by @Datseris and we can do the same. I'm actually not sure anymore, in the end even with the current way it is enough to pass dummystep for the agent_step.

Why we can't just document that using this more advanced use of scheduling is allowed and requires to pass a dummystep for the first argument (agent_step)?

All in all this seems to me more a documentation problem than an API problem.

We could even make the agent_step! and model_step! keywords argument when put inside the model with default to dummystep, to let the user not pass dummystep functions

Answer 9 · 2023-09-26T11:44:44.000Z

I am having trouble following what you are saying. Would you mind rephrasing and higlighting what the problem is?

Answer 10 · 2023-09-26T11:48:23.000Z

Why we can't just document that using this more advanced use of scheduling is allowed and requires to pass a dummystep for the first argument (agent_step)?

Don't worry, that's documented already.

All in all this seems to me more a documentation problem than an API problem.

Sorry, I think I disagree. Here's how I see it in a nutshell.

The core argument was that the JuliaDynamics packages could move towards a unified approach to use a single step! function inside the model struct.
It's about transparency of the package's inner workings and less friction for user to move from linear to non-linear models.
The package code could avoid having to deal with two different stepping functions that are pretty redundant except for providing a bit of convenience to first-time users. (And possibly allowing for performance optimisations in multi-agent ABMs. But I'm not sure about this, as I said above.)

I am having trouble following what you are saying. Would you mind rephrasing and higlighting what the problem is?

Yes, please. Maybe I'm misunderstanding your proposal after all.

Answer 11 · 2023-09-26T11:48:24.000Z

yes, it is correct, in v6 there is no real reason to "force the removal" of the agent_step! function. It is given by a keyword. So if it is not the default value dummystep we can easily re-create all previous behaviour. And that is probably the best actually.

Answer 12 · 2023-09-26T11:49:54.000Z

The core argument was that the JuliaDynamics packages could move towards a unified approach to use a single step! function inside the model struct.

No, this was never an argument. The argument was that the dynamic rule would be inside the model. Whether this is one, or two functions, or a completely different data structure alltogether is a whole different story. It it was made clear in this discussion, that there is not going to be any enforcment on the data structure of the dynamic rule to allow for future improvements. Whatever the dynamic rule is for SingleContainerABM, this has no impact on the general Agents.jl package.

Answer 13 · 2023-09-26T11:51:41.000Z

This should be done by the user if he/she removes agents, right?

Answer 14 · 2023-09-26T11:57:54.000Z

No, this was never an argument.

But that's why I started this thread. Why do you think it was never an argument?

The argument was that the dynamic rule would be inside the model.

Could you please elaborate a bit more on the concept of the dynamic rule?

Answer 15 · 2023-09-26T12:07:02.000Z

The dynamic rule is the "collection of data structures that contain the dynamic rule of the system/model: the description of how the model evolves in time".

The reason to move agent_step! in the model is to have the "dynamic rule" (which, in the context of SingleContainerABM is the two functions agent_step!, model_step! and an instance of a scheduler) into the main model data structure. This would make Agents.jl more in line with the rest of the dynamic modeling ecosystem in Julia.

Forcing the dynamic rule to be "a single Julia function" is too restricting and can never happen in Agents.jl generally. We may agree on forcing the dynamic rule to be a single Julia function for the specific model type SingleCOntainerABM, but this has no bearing on other model types we have in the pipeline. In addition, flexibility on the data structural type of the dynamic rule will allow other softwares, such as those discussed in #884 , to possibly interface better with Agents.jl.

So, let's please clarify ourselves. Merging agent-model step in one function is a different topic and argument from including the dynamic rule in the model type. We discuss them in tandem because they must be done in the same PR, not because they are related.

Answer 16 · 2023-09-26T12:09:32.000Z

actually, there are more complications on completely removing the "convenience" of agent_step!. Look here:

https://github.com/JuliaDynamics/Agents.jl/blob/main/src/simulations/step.jl#L45-L58

as you can see, agent iteration can be handled differently depending on the model type.

Answer 17 · 2023-09-26T12:18:25.000Z

there are more complications on completely removing the "convenience" of agent_step!

Yes, very likely there are even more. That's why I wanted to discuss this here again and see if it's possible and/or feasible.

as you can see, agent iteration can be handled differently depending on the model type.

Which only those models benefit from that fall into the category of being so linear that they fit the use of agent_step! and its restrictions in scheduling. So it's of course convenient that users don't have to make sure that they don't modify the iterators they use.

Answer 18 · 2023-09-26T12:23:49.000Z

Just to clarify: by linear you mean models where all agents are activated once per step, but with a dynamic (state-dependent) order?

Because in our schedulers you can also have rather trivially partial activation, where agents that only satisfy some property are activated.

Can you be specific with what linear means? Perhaps you can provide here a (simplified/shortened) version of the model you used in your research, where you said that agents are activated "dynamically"?

Answer 19 · 2023-09-26T12:35:14.000Z

By linear I mean models in which each agent is at most activated once during each step.

Answer 20 · 2023-09-26T13:18:00.000Z

We could alter scheduling @Tortar . The scheduling was anyways made "dynamic" by @AayushSabharwal . We could make the schedulers all create a particular type, which is an iterator that creates the inherent loops in lines https://github.com/JuliaDynamics/Agents.jl/blob/main/src/simulations/step.jl#L45-L58

In this way, the output of schedule(model [, scheduler]) would be a custom iterator that performs the above lines code code: it internally iterates over agents and takes care of the intricacies of the model specific container and the erasure of agents.

This would make things more accessible for merging agent+model stepping.

Answer 21 · 2023-09-26T13:44:35.000Z

Good idea!

(already applied in the PR)

Answer 22 · 2023-09-26T16:14:30.000Z

So what is the status quo? I have updated the PR with the suggestion made by @Datseris and made the agent_step! and model_step! arguments as keywords.

So currently my opinion on all of this is neutral. I think we could keep the current way, and maybe update the docs to pinpoint to a better extent that using just the model_step! is something one could do without problems, maybe even giving some kind of example on how this could be done. Or instead we could just go with the new methodology suggested by @fbanning by default. I don't have really an opinion, to me both seem fine, maybe the convenience argument still stands though, and so just enphasizing more this different way in the docs is fine.

Answer 23 · 2023-09-26T20:40:23.000Z

I have thought about this extensively. I was thinking about it in my bouldering session as well. Really deep thoughts 🗡️

My conclusion is to not do the suggested change. Here are my main points:

The existence of agent_step! is a feature of Agents.jl. It is not only convenient, but it also handles the dynamic removal of agents in the model. It is also optimized on a per-model basis. The reason "other software don't do this", is because other software don't have this feature. Just like they don't have countless other features of Agents.jl.
We have heard often the argument that "real world applications of Agents.jl don't follow the simple rule of linear agent activation". Yet, I still would like to see exactly how, in specificity, are these real world examples structured. The current set up of scheduler+agent_step allows for partial agent activation and dynamic agent activation based on agent states. I honestly have trouble seeing how this would not cover the majority of real world cases. @fbanning can you please tell us specifically how the agent sequencing and activation is performed in your research application? This will help me better understand your use-case.
We can do a better job, already in the Tutorial, discussing that "providing an agent stepping function is not only a convenience, but it also allows handling dynamic removal of agents and utilize various performance optimizations. Nevertheless, it is possible that your model may require special scheduling of agents, in which case ignore the agent stepping function and manually activate agents in the model stepping function.". Or something like that.
And, last but not least, we must not forget the main reason to not do such a change: it is breaking, and will annoy all users that actually used agent_step!. We don't have data on the percentage of these users, but given that this is the way we recommended in the tutorial so far, the percentage has to be the majority.

I agree with @Tortar 's comment that

All in all this seems to me more a documentation problem than an API problem.

The existence of agent_step! was perfectly fine, and also perfectly allowed for the case that this Issue wants: the non-existence of agent_step!. Simply don't use it. Especially now that agent_step! becomes a keyword, things are even simpler, as you don't even have to pass in dummystep. It happens for you automatically. I do not agree with the counter-argument to the above:

It's about transparency of the package's inner workings and less friction for user to move from linear to non-linear models.

because it feels weird to me to apply this argument in this particular case only. We are also not transparent about the inner workings of e.g., finding nearest neighbors, or how the random sampling of single agents work, or the majority of more complex functionalities like e.g., OSM space.

I also want to comment on this argument:

Explaining scheduling to users is not that complicated (it's all just a bit of [filtered] iteration anyways) but it would (i) enable them to move from simple to complicated models with less friction, and (ii) allow us to simplify the stepping code because we don't need to differentiate between different types of stepping functions and their desired order anymore.

But we anyways explain scheduling. We have a dedicated API section on scheduling. Besides, this argument favors/establishes that indeed this is more a documentation issue than an API issue. Secondly, I don't see how this "simplifies" the stepping code or would help users to move to more complex models. A user that codes complex models surely would not be daunted by the rather trivial concept of ignoring the agent-stepping function.

I think all of this can be solved by simply being more transparent about scheduling in the Tutorial. Explicitly point to the scheduling API section and say "look, agents are scheduled by doing simply schedule(model). See [Scheduling] for more.".

(also, but this is my opinion, not something factual, I personally thing that developing a model becomes more complex by removing the existence of the agent stepping function. Not less complex. Because, if you can utilize it, things are easier. If you can't utilize it, well, nothing changes, you were anyways passing dummystep in, and in v6 this happens by default so you have absolutely no change in your development process)

So in conclusion, I see where @fbanning is coming from. But I would argue that the majority of @fbanning concerns can be addressed by improving the documentation, not by reducing the amount of features that Agents.jl has. Because, throughout this discussion, it became very clear to me that the agent-step is a feature. Most features are "conveniences" after all.

Answer 24 · 2023-09-26T20:40:52.000Z

and @fbanning thanks a lot for challenging the status quo, this is always important in improving a software. Feel free to counter-argue to my arguments above if you disagree.

Answer 25 · 2023-09-26T20:45:31.000Z

@fbanning also note, in v6 the instruction to the users is: "you need to provide at least one of the agent- or model-stepping functions. If an agent-stepping function is not provided, it is assumed that all dynamics, including scheduling and activating agents, happens in the model-stepping function."

So I would argue that this can address one of your concerns of there being a disconnect between the Tutorial/introductory usage, and real-world usage.

Answer 26 · 2023-09-26T22:32:36.000Z

I was thinking about it in my bouldering session as well. Really deep thoughts 🗡️

For this reason, I couldn't agree more (maybe actually I could if you had sent a V10 in your session) :D

Answer 27 · 2023-10-05T19:54:40.000Z

Closed in #889 . The new tutorial explains this very well, and besides, agent_step! is now dummystep by default.