Skip to content
This repository has been archived by the owner on Jun 21, 2024. It is now read-only.

Add support for GraphSpace in abmplot recipe #114

Merged
merged 42 commits into from
Feb 2, 2023
Merged

Conversation

fbanning
Copy link
Member

@fbanning fbanning commented Nov 2, 2022

Closes #11

The basics are working already.

Screenshot_20221102_214657

I'll try to finish this in the upcoming days so that it works nicely with user-styled vertices and edges.

@fbanning fbanning self-assigned this Nov 2, 2022
@fbanning
Copy link
Member Author

fbanning commented Nov 2, 2022

Some to-dos:

  • Hide axis labels for GraphSpace plots
  • Implement dynamic styling option for vertices and edges
  • Add SIR model to example/ for testing
  • Test with examples from examples/
  • Decide what to do regarding keywords (see comment below)

@fbanning
Copy link
Member Author

fbanning commented Nov 14, 2022

Hey @Datseris , a design question for you: The GraphSpace plots behave a bit differently than the rest because we have not just agents (e.g. scatter points) like in the other spaces. Graphs consist of vertices and edges, therefore we need to allow users to style them appropriately.

My current implementation uses the ac/as/am kwargs to style the vertices of the resulting graph, as that's the behaviour that people might expect. The edges are then styled with graphplotkwargs = (; edge_color = foo, edge_width = bar) and this is handled appropriately in the recipe to account for the possibility of passing functions as usual.

Now my question is whether this seems sane to you? There's certainly a break in the UX here but I'm not sure how to solve for it. It boils down to these two options:

abmplot(model; ac = node_color, as = node_size, graphplotkwargs = (; edge_color, edge_width))
# or
abmplot(model; graphplotkwargs = (; node_color, node_size, edge_color, edge_width))

Which do you prefer? The first version is closer to the regular behaviour of our abmplot. The second version is closer to the regular behaviour of GraphMakie's graphplot but might cause confusion for users expecting to be able to use the regular abmplot kwargs.

@fbanning
Copy link
Member Author

Other than that this PR just needs to add some documentation, increment patch number, etc.

Here's a little (admittedly weird) showcase of our SIR model with random colours and widths of the edges:

abmvideo.mp4

@fbanning
Copy link
Member Author

ac/as/am kwargs

Maybe it would be a good idea to drop them completely and call them color/markersize/marker as it's done in most other recipes as well? 🤔

@fbanning
Copy link
Member Author

I've rearranged lifting.jl to make use of method dispatch instead of nested if/else statements for type checking inside the function body. This was done because it started to get messy with all those extra if/else statements for special cases (which space?, function or not?, etc.). It also reduced LOC, makes it easier to find the case I'm looking for while debugging, and will allow for easier addition of more special cases later on. It's a win-win-win situation I think.

- Add method to pass an existing ABMObservable instead of an ABM
- remove unnecessary _add_interaction kwarg
- Nest different calls to abmplot/abmplot! to avoid code duplication
- reorder some things in recipe to make more sense
- Works again with new recipe structure
- Added support for GraphSpace inspection
- Refactored id collection into separate ids_to_inspect methods
- Changed GridSpace to AbstractGridSpace to also support GridSpaceSingle ootb
@fbanning fbanning marked this pull request as ready for review January 23, 2023 17:36
@fbanning
Copy link
Member Author

@Datseris Please have a final look if you're happy with everything. :)

@Datseris
Copy link
Member

I am sorry for taking so long. Yes this is good to go, but did you add something about this in the docstring of abmplot? That am, ac, as are functions of vectors of agents if the plotting function is called on a graph space model?

@Datseris
Copy link
Member

Eh, here we have city_color(model, idx) and ac = city_color. What does this mean...? What I thought was that for the size, markerstyle, and color, if the user provides function, the function takes as input all agents in the current node. (At least, this is what we did in the pre-Makie days where a graph space recipe existed).

In any case, this needs documentation, I don't see anywhere documnted in a docstring how to change edges etc. at the moment., but all of these are possible to do as shown in the example file. Probably worth adding a subsection ## GraphSpace recipe or something?

@fbanning
Copy link
Member Author

fbanning commented Jan 30, 2023

Thanks for the feedback. Yeah, you're right, I should add documentation to explain the details of how to use the recipe with GraphSpace models. A subsection definitely makes sense and I'll add that as soon as I find the time.

Eh, here we have city_color(model, idx) and ac = city_color. What does this mean...? What I thought was that for the size, markerstyle, and color, if the user provides function, the function takes as input all agents in the current node. (At least, this is what we did in the pre-Makie days where a graph space recipe existed).

The function does that internally. It iterates over space.stored_ids which is a vector of vectors of ids in this position (see line 17). The user just provides a function that does something with each of the ids in a given position as shown in the example file. Did we handle this differently in the earlier versions of the graphspace recipe?

I chose idx as argument name for the example because it's the index of a node in the graph, indicating that we are creating one color/marker/markersize per node. While I prefer the graph based vocabulary here (since it's a GraphSpace model after all), I see that this might cause confusion between agent identifier id and node index idx. Internally it can stay as it is but for the documentation and the examples in there, I think I will switch this to pos instead, just to make things a bit clearer to the users.

I don't see anywhere documnted in a docstring how to change edges etc. at the moment., but all of these are possible to do as shown in the example file.

edge_color and all other edge related keywords are done via GraphMakie.jl. This means that the user just needs to understand how to use it to also be able to use our recipe. (See lines 107-111 for how I've handled this internally.) However, I'll definitely make sure to document this properly as well and link to the GraphMakie.jl docs for further explanation on how to tweak the resulting graph plot. :)

@Datseris
Copy link
Member

The function does that internally. It iterates over space.stored_ids which is a vector of vectors of ids in this position (see line 17).

Okay, I see. Yes, this is different as it was done before. In the original version, the approach was: as, ac, am are functions that take in an iterable of agents and output a size, color, marker. The iterable is the agents in each given node, so literally agents_in_position(pos, model) for each pos. This means if you wanted the size to be the number of agents you'd pass as = length. If you wanted the color to be the proprtion of infected you'd do ac(agents) = count(isinfected, agents)/length(agents). It seems like an approach that remains harmonious with the other spaces, because, the ac, am, as arguments are only about the agent, not about the model. Here they remain as such without accepting the model as the input.

@fbanning
Copy link
Member Author

OK, I'll have to think about how to do this.

For the edges it will of course not work because they follow such a different concept (i.e. the connection between two positions). There we will still need a special approach but that's not breaking for anyone since it's really only used with GraphSpace models.

@fbanning
Copy link
Member Author

fbanning commented Feb 1, 2023

OK, now it works like this

as(agents_here) = length(agents_here)

and that should be what you want, right? No more model or position or anything else. Just a function that iterates over the agents in a given position.

@Datseris Datseris merged commit 30f1322 into main Feb 2, 2023
@Datseris Datseris deleted the graphspace-support branch February 2, 2023 11:01
@Datseris
Copy link
Member

Datseris commented Feb 2, 2023

Thanks so much for his @fbanning ! Tagging a release now!

@Datseris
Copy link
Member

Datseris commented Feb 2, 2023

@fbanning I think you need to update the graph makie dependency in OSMMakie for us to tag a new release, see here: https://github.com/JuliaRegistries/General/actions/runs/4074308616/jobs/7019298171#step:16:186

@fbanning
Copy link
Member Author

fbanning commented Feb 3, 2023

Yeah, didn't tag the new release so far. Will do that now, should be available soon ™️ .

@fbanning
Copy link
Member Author

fbanning commented Feb 3, 2023

Done. Already bumped OSMMakie compat on here as well as fixing the changes needed for Makie 0.19 to work (they renamed textsize attribute to fontsize).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Agents Concerning plotting of Agents.jl models enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GraphSpace support for agent based models
2 participants