Refactor recipes

[fbanning]

• Refactor recipe system to dispatch on model.space
• Rename and reorganise files in /src/agents/ to better reflect what they're doing and keep them more functionally separate from each other
• Refactor OpenStreetMapSpace into recipe system with osmplot as static_preplot! and agents as scatter! plot
• Remove ABMStepper struct and functionality and move all of it into Attributes of ABMPlot recipe
• Move all former functionality of abm_plot, abm_play, and abm_data_exploration into ABMPlot recipe
  • abm_plot
  • abm_play
  • abm_data_exploration
• Update abm_video convenience function to work with ABMPlot recipe
• Add inspectable::Bool Attribute to recipe which controls if agent inspection on mouse hover should be enabled or not
• Add convenience function akin to former abm_data_exploration
• Test all examples without and with controls, without and with data plots, without and with inspection
  • flocking (2D continuous with polygons)
  • daisyworld (2D grid)
  • schelling3d (3D grid)
  • zombies (osm)
• Documentation. All of it.
• Update Makie compat entry to 0.16. (Currently blocked: We depend on OSMMakie and subsequently on GraphMakie which in turn waits for the release of Makie 0.16.4 that fixes one of their bugs. Need to be a bit patient.)

[Datseris]

@fbanning can you explain to me something which I don't get from the recipe system... There is both a function abm_plot but also abmplot...? Why?

The other thing I don't understand is where is abmplot actually defined. It isn't here: @recipe(ABMPlot, model) do scene, it isn't here: function Makie.plot!(abmplot::ABMPlot{<:Tuple{<:Agents.ABM{<:GridOrContinuous}}}),. Like, we are calling a function abmplot! but I don't find its definition.

[fbanning]

@fbanning can you explain to me something which I don't get from the recipe system... There is both a function abm_plot but also abmplot...? Why?

The similar names abmplot and abm_plot are a bit unfortunate, yes, but couldn't really be avoided because I definitely wanted the underlying recipe to have a "telling name" (ABMPlot as its type and abmplot/abmplot! as the corresponding functions).

While abmplot and abm_plot are similar by name, their purpose is still quite different. I'll try to explain what they do:

abmplot is exported via the recipe system and takes in model<:ABM as an argument. You can see it as a new plot type which is comprised of some other plot types or primitives (e.g. multiple lines in the case of linesegments). So in the most basic case of a 2D GridSpace calling our abmplot function will just create a scatter plot of the model with the given agents positions (normally that would be pos = abmstepper.pos).

abm_plot is what already existed before and does not only return the figure but also the abmstepper. This is necessary to update the plots. If users would only use something like abmplot(model; pos = [model[id].pos for pos in ids]), the plots won't update. But you know all of this because this behaviour is just how it always was.
Users are advised in our docs to use abm_plot, abm_play, and abm_data_exploration because these functions conveniently construct the ABMStepper from a given model, connect it to the newly created plot and return both the figure and abmstepper for later use.

The other thing I don't understand is where is abmplot actually defined. It isn't here: @recipe(ABMPlot, model) do scene, it isn't here: function Makie.plot!(abmplot::ABMPlot{<:Tuple{<:Agents.ABM{<:GridOrContinuous}}}),. Like, we are calling a function abmplot! but I don't find its definition.

That's just the way Makie recipes work: They have two parts. You need to define both a @recipe block for the plot type and its attributes as well as a custom plot! function to define the actual functionality of the resulting plot function.
I really can't explain it any better than it's done here: https://makie.juliaplots.org/stable/documentation/recipes/index.html#full_recipes_with_the_recipe_macro
Reading just that paragraph (up until the example) should hopefully clear things up for you. If it doesn't, feel free to ask again.

[fbanning]

Even though the above comment was already pretty long (it's really hard to explain these things without going into too much detail), let me add another thought on the first topic: It's possible that the overlapping and maybe somewhat confusing naming can be fixed. All while making our code base smaller and providing a better user experience. Too good to be true? Maybe.

In its current state, abmplot is static/unusable without first converting the model data into Observables stored in ABMStepper and linking its fields to the plot. But as I've already hinted at in the initial PR introducing recipes (see the Further Ideas section in the initial post of #72), I think it's possible to get rid of ABMStepper. Instead we could rely only on the function args/kwargs defined in the recipe because all of them are automatically converted into Observables. Their values would have to lift the values from the only argument provided to the function, namely model<:ABM.

If this is indeed possible, then some massive simplification of our code would follow. We could remove ABMStepper, its specific stepping functions as well as the need for abm_plot, abm_play, and abm_data_exploration. Instead we could handle those cases via keywords (= Attributes) of the recipe system (sth. like enable_controls = true and enable_data_exploration = true).

Then the usage would look as follows:

#formerly abm_plot
abmplot(model)

#formerly abm_play
abmplot(model; enable_controls = true)

#formerly abm_data_exploration
abmplot(model; enable_data_exploration = true)

Such a user experience would be highly desirable to me. It gives the user one single function to work with and allows custom usage via the keywords.

I want to try out this approach right after I've finished this PR but so far I didn't find the time or muse to try it out. But let's do this step by step. Let's first finish this PR. Then I'll work on the next step to (hopefully) simplify our plotting a bit more.

[Datseris]

Thanks for the detailed reply, I understand what is going on now. Notice that the necessity of ABMStepper comes not only from creating the observables and carrying them around as a container, but also because of Julia's multiple dispatch system. You need a way for the central step! function to trigger the observable updates without extra lines from the user, and this is exactly what step!(::ABMStepper d0es.

EDIT: But, given the recipe system, we can merge the structs ABMPlot and ABMStepper, right? We need the later to define dispatch on step!, and we need the former to define dispatch on Makie's plotting system, so in essense we need 1 struct that defines 2 dispaches?

EDIT 2: But, we should be careful: if this is indeed possible, we should only do it if it increases code clarity. The important point is not to minimize lines of source code, but to maximize how easy it is to understand the code. Sometimes the extra lines help with that.

[Datseris]

Refactor OpenStreetMapSpace into recipe system with osmplot as static_preplot! and agents as scatter! plot

Wait, I'm a bit lost now. The refactoring into recipes shouldn't really care about what kind of space we have right? That's exactly why I wrote the function plot_agents_space!. This is the function that dispaches on space, but I see now already that ABMPlot also dispatches on space type. But why can't it just call plot_agent_space!?

EDIT: I stand corrected. The plot recipe dispaches on the position type not the space. But, same argument: the position type from open street map space is also Point2f0, so it shouldn't care?

[fbanning]

the position type from open street map space is also Point2f0, so it shouldn't care?

I guess you're right. Will merge the two methods.

We will definitely need a new method for GraphSpace later on though because it handles things so differently.

[fbanning]

given the recipe system, we can merge the structs ABMPlot and ABMStepper, right?

I think so, yes.

We need the later to define dispatch on step!, and we need the former to define dispatch on Makie's plotting system

Yes, that's the status quo. My hope is that we can eliminate the need for a separate ABMStepper struct completely. This should be possible if we define all the necessary ABMStepper variables as attributes inside the recipe via lift. We should then be able to only update the value of the model argument (abmplot.model[] = model) after every step and the plot should update accordingly. Remember though, this is untested and just makes sense in theory in my head right now. Time will tell if it's really doable.

we should only do it if it increases code clarity. The important point is not to minimize lines of source code, but to maximize how easy it is to understand the code. Sometimes the extra lines help with that.

Absolutely agree with this but in addition I want to emphasise the importance of a good user experience. Let's see how this turns out once I start working on it. But first let's finish this PR.

[Datseris]

Yes, GraphSpace needs to be handled differently, but that's a different PR.

abmplot.model[] = model

That seems like a great approach. Lifting everything from a model observable, including the position and all other stuff. Do you think you can do this in this PR? I'd suggest to do this in the ABMSTepper first, while still keeping a separate ABMPlot, as is right now. Then, after we have this large convenience change of only having to update abmstepper.model[] = model, instead having to call step!(abmstepper, ...), we can think about how to merge the stepper and plot objects.

Regarding user experience: I think your suggestion is anyways simpler for a user. It also gives them the possibiolity of passing a new model instance instead of having to recreate a plot from scratch. Also, it simplifies a lot our "reset" button functionality.

[fbanning]

OK, everything seems to work now. I'm locally building the docs to double check that everything looks good.

[fbanning]

All fine. Tag, merge, party.

[AayushSabharwal]

I haven't had the time to keep track of much here or in Agents for a while now... Looking forward to playing with the new plotting once it's out