Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring the access to space related functions to move towards multispace models #910

Open
Tortar opened this issue Oct 21, 2023 · 4 comments
Labels
design Design-related discussions for APIs, Interfaces, etc. model Related to an existing or new model implementation

Comments

@Tortar
Copy link
Member

Tortar commented Oct 21, 2023

Take this function:

function id_in_position(pos, model::ABM{<:GridSpaceSingle})
    return abmspace(model).stored_ids[pos...]
end

we could refactor this code so that to have:

id_in_position(pos, model::ABM) = id_in_position(pos, abmspace(model), model) # this can be actually done only one time, probably
                                                                              # to put in space_interaction_API.jl
function id_in_position(pos, space::GridSpaceSingle, ::ABM)
    return space.stored_ids[pos...]
end

What does this allow? It gets us nearer to possible implementation of multispace models since we need to give the possibility to the user to access the space-related function dispatching on the space when implementing a multi-space extension (which can be just as simple as a namedtuple of spaces), so that to allow to refer to different spaces inside his/her code.

Note that this is a costless abstraction.

cc: @Datseris

Related to #719, #720

@Tortar Tortar added design Design-related discussions for APIs, Interfaces, etc. model Related to an existing or new model implementation enhancement New feature or request and removed enhancement New feature or request labels Oct 21, 2023
@Datseris
Copy link
Member

It gets us nearer to possible implementation of multispace models

For me this needs to be justified in the first place... How general is the need for this functionality?

But yes, in any case, the refactor outlined here is very easy to do and also non breaking.

@Datseris
Copy link
Member

However, the proposal here doesn't work for two spaces of the same kind, two different grid spaces. Which I would assume would be the most common case of this already extremely uncommon feature.

@Tortar
Copy link
Member Author

Tortar commented Oct 21, 2023

It works even for two space of the same kind, take id_in_position , now its signature is id_in_position(pos, model), we will change it in id_in_position(pos, [, space], model) so that the user can pass id_in_position(pos, grid1, model), id_in_position(pos, grid2, model),...

For me this needs to be justified in the first place... How general is the need for this functionality?

We will solve #719 for example. More generally we will allow for separating types in different spaces which can be useful sometimes for performance e.g. the wolfsheep model would be faster this way. Also some mixture of space types could be actually interesting e.g. the one suggested in #719 is already an example NoSpace + Any other Space

@Datseris
Copy link
Member

Well, yes, but as I also said in #719 I am not at all convinced about the usefulness or necessity of this feature given the arguments of #719 as it is arguably simpler to achieve the same result without mixing spaces.

More generally we will allow for separating types in different spaces which can be useful sometimes for performance e.g. the wolfsheep model would be faster this way

There is no proof yet of whether this will have measurable performance boost, because there is no way for multiple dispatch to deduce the type. GridSpaces do not store agent types nor have any knowledge of them. So type instability will remain the main problem.

Notice also that it is more complicated to setup this versus the setup of just 1 space. And more complicated to handle tit as well. All add_agent! functions will need to be modified. add_agent! is already the most complex function of the package when it comes to input type dispatch, having position, agent type, model, and keywords. Now you would need to add one more level of complexity: the space to add the agent. All of this takes a lot of development effort and a big deal of mental arithemtic from the user to keep track of what goes to which space all the time. So, you need to be aware, and always have in mind the metric we care about: time from 0 to paper. This time includes not only the compitational performance, but also the time for the user to develop and analyze their model, which in many cases takes much more than actually running the code.

That being said, nothing stops this from being an optional feature that is discussed at some advanced section of the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design-related discussions for APIs, Interfaces, etc. model Related to an existing or new model implementation
Projects
None yet
Development

No branches or pull requests

2 participants