In a GridSpace model I would like to write
node_neighbors(agent.pos, model) |>
neighbors -> get_node_agents(neighbors, model)
but there is no method for get_node_agents (and respectively for get_node_contents) to get all the agents on a list of nodes.
Is there a way to collect agents from a list? If yes, what is it called? If not, would it make sense to implement such a method?
Here's the error message for your further information:
ERROR: LoadError: MethodError: no method matching get_node_contents(::Array{Tuple{Int64,Int64},1}, ::AgentBasedModel{MyAgent,GridSpace{SimpleGraph{Int64},2,Int64},typeof(random_activation),Dict{Symbol,Any}})
Closest candidates are:
get_node_contents(::Integer, ::AgentBasedModel{A,var"#s57",F,P} where P where F where var"#s57"<:Agents.DiscreteSpace) where A at C:\Users\Frederik Banning\.julia\packages\Agents\kEF2y\src\core\discrete_space.jl:342
get_node_contents(::AbstractAgent, ::AgentBasedModel{A,var"#s57",F,P} where P where F where var"#s57"<:Agents.DiscreteSpace) where A at C:\Users\Frederik Banning\.julia\packages\Agents\kEF2y\src\core\discrete_space.jl:349
get_node_contents(::Tuple, ::AgentBasedModel{A,var"#s57",F,P} where P where F where var"#s57"<:Agents.DiscreteSpace) where A at C:\Users\Frederik Banning\.julia\packages\Agents\kEF2y\src\core\discrete_space.jl:351
Are you looking for space_neighbors? If not, I think I'm missing some specifics in you question. Could you write up a little example you'd expect to work?
space_neighbors is similar to get_node_contents as it returns a list of agent ids. I would like to collect a list of agents directly as it is done in get_node_agents. Is this possible?
If I understand correctly now, this could be done currently with
[model[n] for n in space_neighbors(agent,model)]
is that what you're looking for?
If so, it's an easy extension to add—and perhaps a useful one to keep some consistency across the API. Will discuss with the other devs once you confirm if this is / isn't what you're after.
Edit:
node_neighbors(agent.pos, model) |>
neighbors -> [get_node_agents(n, model) for n in neighbors]
works for your explicit construction.
Yeah we can add a get_node_agents method for grid space, we probably forgot it :P @fbanning it is really simple to add this method if you want to increase your contributions. By using the vertex2coord or coord2vertex function. If you have a look at the source of space_neighbors we pretty much do the same thing. Same for node_neighbors.
Wait, I misunderstood. Why in your case broadcasting isn't an option? instead of get_node_agents(neighbors, model) do get_node_agents.(neighbors, Ref(model)) ?
[model[n] for n in space_neighbors(agent,model)]
Yep, that's exactly how I do it right now. As you rightfuly pointed out, my question is more related to coherence in the API and if we should add a corresponding method.
get_node_agents.(neighbors, Ref(model))
Yes, broadcasting like this works but it does not really feel in line with the rest of the API because it not only requires broadcasting (which could be expected knowledge from Julia programmers) but also requires a change in the parameters of the get_node_agents function (model -> Ref(model)). I think in terms of usability and coherence, we should aim at providing multiple methods for the normally provided API functions.
If you don't have any objections, I will make a quick PR for such an extension of get_node_agents.
Sounds good—go ahead.
I just ran some @btime tests for the three versions to find out which one would be more efficient.
julia
[model[n] for n in space_neighbors(agent,model)]
julia
node_neighbors(agent.pos, model) |>
neighbors -> get_node_agents.(neighbors, Ref(model))
julia
node_neighbors(agent.pos, model) |>
neighbors -> [get_node_agents(n, model) for n in neighbors]
There are n agents on m neighboring nodes. It seems options 2 and 3 don't quite return what I expected (n-element Array{MyAgent,1}) but instead it returned a m-element Array{Array{MyAgent,1},1}. This is basically an Array of one Array per neighboring nodes. Option 1 does return a simple n-element Array{MyAgent,1} which is what people most likely want to use to iterate over. This is also what is returned by get_node_agents when only one node (instead of a list of nodes) is given as function parameter. Therefore, if we want to extend get_node_agents a further consolidation of the m-element Array is needed to get the desired outcome of an n-element Array.
Regarding performance:
To sum up: I would go ahead and implement two method extensions. One for space_neighbors and one for get_node_agents. By that the user will have the free choice to use whichever approach they prefer.
What I see 1. doing is obtaining a list of all agents that surround agent based on the type of space, and the grid radius (with a default of r=1 here).
The other two provide a list of agents in each direction that surround the agent. That particular information is lost in the space_neighbors version—which is what I was trying to get at in the above discussion: what are you really looking for in this use case?
It will be good to continue this discussion on the PR though. Then we will have some tests, and examples to play around with such that we can design this better.
Yes, you're right. I agree that those are different use cases.
I think it would be best if space_neighbors returns an n-element Array{MyAgent,1} (omitting directional information) and if get_node_agents returns an m-element Array{Array{MyAgent,1},1} (preserving directional information).
Edit: Which I think means that no extension of space_neighbors seems to be needed anymore.
@fbanning, I'm doing a big rewrite in #317. I'm still not sure what you're really after in this issue, so here's a summary of what we now have, tell me if I'm missing something.
Since this issue and my related PR have collided greatly with the rewrite of the whole API, I don't have much more to add here.
The functions you mentioned and what they return seem totally fine to me. "What I was really after" in this issue was that I wanted to get the contents of a list of positions (either in the form of ids or agents). As far as I can see, the proposed _ in_position functions will allow to do exactly that with broadcasting.
OK, great—thanks.
Yeah, sorry. We're doing a large churn at the moment ready to release 4.0 and we're all under some time pressure to get it done sooner rather than later.
Closed via #317. Feel free to open something else up if the new API still doesn't sort out your issues in this area. There may be some churn still to go before 4.0 though.
@fbanning would be nice to have a look at current master docs that reflect upcoming 4.0; any comments are appreciated.
Most helpful comment
Yep, that's exactly how I do it right now. As you rightfuly pointed out, my question is more related to coherence in the API and if we should add a corresponding method.
Yes, broadcasting like this works but it does not really feel in line with the rest of the API because it not only requires broadcasting (which could be expected knowledge from Julia programmers) but also requires a change in the parameters of the
get_node_agentsfunction (model->Ref(model)). I think in terms of usability and coherence, we should aim at providing multiple methods for the normally provided API functions.If you don't have any objections, I will make a quick PR for such an extension of
get_node_agents.