Allow using NoSpaceAgent in spatial models
#720
Conversation
Fixes issue JuliaDynamics#715. With thanks to Michael Hatherly!
`@agent` can now take a docstring directly, instead of relying on an additional call to `@doc`.
|
Sorry, this still includes my commits from #717. I'm not sure how to get rid of them, and only show commits for this issue? |
Codecov Report
@@ Coverage Diff @@
## main #720 +/- ##
=======================================
Coverage 89.34% 89.35%
=======================================
Files 27 27
Lines 1830 1831 +1
=======================================
+ Hits 1635 1636 +1
Misses 195 195
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| if any(isequal(:pos), fieldnames(A)) | ||
| newagent = A(id, pos, properties...; kwargs...) | ||
| add_agent_pos!(newagent, model) |
There was a problem hiding this comment.
Wait, isn't this a hit to performance? Please check if there is a more efficient way to test if a struct as a field pos.
There was a problem hiding this comment.
| if any(isequal(:pos), fieldnames(A)) | |
| newagent = A(id, pos, properties...; kwargs...) | |
| add_agent_pos!(newagent, model) | |
| if :pos in fieldnames(A) | |
| newagent = A(id, pos, properties...; kwargs...) | |
| add_agent_pos!(newagent, model) |
I believe this is the idiomatic way of doing this.
You need to update your own |
|
Thanks for your comments. I'm on my Christmas break now, but will look into this again come January. Merry Christmas to you 🙂 🎄 |
|
to you too!!! |
|
bump |
|
Hi @Datseris, thanks for the reminder and sorry for the delay. Apart from having to take care of other tasks, I was a bit unsure about how to proceed with this issue. On the one hand, I still think that it would be useful to allow non-spatial agents in spatial models, for example for institutions that act as agents but have no localisation in the landscape. On the other hand, I realised that this is trickier than I thought, due to the underlying implementation. What I hadn't understood is that adding and removing agents is dispatched not based on the agent type, but on the model space type. This makes sense because the However, dispatching based on model space type implicitly assumes that all agents in the model match the model space type. Circumventing this would require checking an agent's I'm not sure what you think about this? For the time being, I had more urgent things that needed dealing with, and so didn't pursue this further. But perhaps somebody has a good idea for a cleaner solution? |
If that's your reason for allowing this, then we shouldn't allow this. Simply do not use the position of institutions that act as agents but their position doesn't matter. Give them some random position. Is there any downside in that? |
|
That's what I'm doing at the moment. It means you have to do a bit more work when looking for nearby agents, because you then have to filter by agent type. Although that is something I have to do anyways with my multi-agent model, so actually for my particular use-case the issue of non-spatial agents is not as pressing as I thought at first. Though I still think that the current behaviour is unexpected from a UX perspective and may cause problems in other situations. (On that note, it may be a sensible addition to the API to allow |
|
I admit, I still do not full follow your process. Why are these "entities without a position" agents in the first place? Why aren't they a model property? |
|
bump here. Is this PR actually necessary? Is there any reason to ever allow models with e.g., continuous space, to accept agents without a position? What's the reason for doing this? I argue above that whatever an agent without position is, it is more likely suited to be a model property instead. Are there any counter arguments to that? If there aren't any, we can close both this PR and #719 . cc @AayushSabharwal @Tortar I would like some thoughts on the matter. |
|
I think a counter-argument is related to scheduling the agents, suppose one wants some agents not on the space, but that they are activated randomly together with the spatial agents, this is enough in my opinion to explain the change, currently a nearby search in this situation requires a filtering of actually non-spatial agents which has to be implemented as spatial as @veddox said which is less performant and more clumsy |
|
Okay then this has to be rebased to current |
|
I actually think to the contrary. If some agents don't have any spatial meaning, can't they just be moved to It's possible that if the non-spatial agents are localized to a corner of the space, they shouldn't come up in nearby searches very often. If they do, the cost could be considered as amortized over the cost of other such searches where they don't come up. Additionally, suppose there is a measurable performance penalty in doing these searches, does it outweigh the cost of dynamic dispatch during stepping since the agent type is not concrete any more? Inherently any such model would face dynamic dispatch unless the stepping is done manually through Are we also sure that allowing this kind of mixture won't cause any bugs, complicated behavior, or edge cases later on? And if we do decide to go along this route, why not go the full distance and allow a mixture of spaces in a model? That feels like a generalization of this problem, and likely useful in more scenarios. These are just my views, and if everyone else thinks differently I'm more than happy to continue in the agreed-upon direction. If there's something I'm missing, please point me in the right direction. |
Not really necessary, these "alternative beings" (not agents) can be a model property. There is no reason to have something within Agents.jl that is not inside the ABM. |
|
I agree with your views. Allowing non-space agents and space agents together does not seem like a wise design decision. This seems like a trick we are trying to do to accommodate a behavior that anyways have alternative solutions already possible with the Agents.jl API: put the agents all in one spot and skip them or don't make them agents at all but rather a model property that is a vector of structs of "buildings" or whatever else the structs are. |
|
I need to say that now that I'm considering your points it seems also to me
better the "normal" way of putting agents in one spot, but the point you
made about multispace models is nonetheless valid anyway, I also considered
some time ago this extension as beneficial. I think that only a big and
generic change like this of multispace models is justified. So I think that
we all three agree about the matter in the end
Il Gio 8 Giu 2023, 11:35 George Datseris ***@***.***> ha
scritto:
… I agree with your views. Allowing non-space agents and space agents
together does not seem like a wise design decision. This seems like a trick
we are trying to do to accommodate a behavior that anyways have alternative
solutions already possible with the Agents.jl API: put the agents all in
one spot and skip them or don't make them agents at all but rather a model
property that is a vector of structs of "buildings" or whatever else the
structs are.
—
Reply to this email directly, view it on GitHub
<#720 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AQH6VX4NKEQ4NTSJYZOCRJ3XKGME5ANCNFSM6AAAAAATFPYFYQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Hi everyone, thanks for weighing in. From the perspective of somebody working with multi-agent models, being able to combine spatial and non-spatial agents can make a lot of sense. Moving "non-spatial" agents to a dedicated position is a possible work-around, but it is quite a kludge. (Depending on the scenario, using model properties could be a clean solution.) However, like I wrote above, I understand that implementing multi-space models is not trivial and would probably incur complexity and performance costs. I also don't have the capacity at the moment to really dig into this myself. So I would be fine with closing the issue and the PR. |
|
Is there a place in the docs which talks about multispatial models like
there is for multi agent-type models?
More people in the future might be interested in doing the same thing and
will run into the same issue. The answer should be easily visible in the
docs, probably in the Space section itself.
…On Mon, 12 Jun 2023, 20:40 Daniel Vedder, ***@***.***> wrote:
Hi everyone, thanks for weighing in. From the perspective of somebody
working with multi-agent models, being able to combine spatial and
non-spatial agents can make a lot of sense. Moving "non-spatial" agents to
a dedicated position is a possible work-around, but it is quite a kludge.
(Depending on the scenario, using model properties could be a clean
solution.)
However, like I wrote above, I understand that implementing multi-space
models is not trivial and would probably incur complexity and performance
costs. I also don't have the capacity at the moment to really dig into this
myself. So I would be fine with closing the issue and the PR.
—
Reply to this email directly, view it on GitHub
<#720 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANTLNXSU4YNZ7WOIYQB7OJDXK5PAVANCNFSM6AAAAAATFPYFYQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Well, no, because multi-space models in Agent.jl are not possible while multi-agent models are. |
|
Right, but I'm just saying that we should write a little note about them in the docs in case someone wants to do multi-spaced stuff and then can't. We should justify why not and what some workarounds are. |
You still haven't answer the following though :
What is the downside of doing that? if anything, this version would give you higher performane as you would not have to filter the neighbor agent searches. The agents that aren't really agents are "activated" in a model stepping function as they should. Answering this question is crucial to understand whether a need for this feature exists or not. |
|
Looking again at this proposal and I have to say there is probably a trivial solution if I understand it correctly: use two agent types and add the second agent type (the one without a position) with |
|
No, |
|
we could also not allow using add_agent_to_space directly, but add_agent_to_model, and its documented usage should be what is discussed in this issue i.e. to use NoSpaceAgents along with SpaceAgents. We can even call it in another way, but its effect should be this one. But there is a simpler way: just extend add_agent! for a NoSpaceAgent when the type of the space is is defined. And that should be enough. |
|
I think we should just close this and just not allow this behavior. In my eyes there has not been enough convincing argument for why allowing this complex behavior, given that you can achieve the same effect with different, and much simpler, alternatives that are already possible such as:
|
|
Your first solution doesn't allow to use the Agents.jl API for those agents if represented as a property. Your second solution can be error prone, and it actually creates a In the end even if it is probably possible to circumvent the issue, if done rightly it should be a 10 lines of code change with no perf penalty for anything already defined which I think is acceptable. |
|
So to summarize the convincing arguments to me are:
edit: mmh, the last part of my comment made me realize that there is some sort of perf penalty anyway I guess in allowing this. But anyway seems not trivial to extend |
|
Well, based on the discussion it appears to me that simply not using the position of these special agents is the way to go. These "alternate agent beings" would anyways must have a different type. Hence, you anyways must filter by agent type when searching for nearby agents or doing any other operation. The filtering operation happens anyways, hence the argument "oh they are found as nearby neighbors" is not strong enough, as you would anyways have to filter. I conclude with what @AayushSabharwal has originally argued: the simplest solution is by far to put all these special agents at a special position, (1,1) or whatever. There is pretty much zero performance impact on this, when taking into consideration that a filter operation will necessarily have to happen at some point or another. |
|
Actually I think that the filtering will never take place since if these agents are not added to the space, the filtering will not be required for spatially explicit operations. |
|
Should we close this now, and also #719, with the final claim: "feature can be achieved with other simpler means that are already readily available from existing API"? |
|
If we're not adding this, then we should definitely document well what these simpler means with existing API are. It's likely that there are others who will want to do the same. Perhaps we can add an "advanced" section which contains a few articles explaining how to do things like this, and then link to the relevant article in the "space" section through a note about putting non-space agents into a space model. |
Fixes #719