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

neighbors #32

Closed
iaciac opened this issue Dec 17, 2021 · 7 comments · Fixed by #63
Closed

neighbors #32

iaciac opened this issue Dec 17, 2021 · 7 comments · Fixed by #63
Labels
improve Make an existing feature better

Comments

@iaciac
Copy link
Collaborator

iaciac commented Dec 17, 2021

I was thinking that, when doing simulations (particularly for games), it is very useful to have the neighbors of a node divided by hyperedge. To explain better, here's what the current neighbors function returns:

hyperedge_list = [[1, 2], [2, 3, 4]]
H.neighbors(2)
# {1, 3, 4}

which is basically the neighbors in the graph projection of H. What I was imagining is actually a higher-order neighbor function that returns {[1], [3, 4]}. This is extremely useful in loops when you want to scroll over the hyperedges attached to a node and then the neighbors in those.

Do you think something like this could be integrated into the current neighbors function? Or maybe two different ones? In my old code I had H.graph_neighbors() and H.neighbors().

@iaciac iaciac added the improve Make an existing feature better label Dec 17, 2021
@leotrs
Copy link
Collaborator

leotrs commented Dec 17, 2021

Agree this would be nice. This sounds like the higher-order version of an egonet, no? So perhaps we can have H.neighbors and H.egonet or something like that.

@nwlandry
Copy link
Collaborator

I like this idea! I also tried to do something similar in another repo of mine (line 126). I would vote to create a dedicated function for this initially, and then if it becomes clear that we can have a single function, then that's great. Renaming to follow your convention is cool with me as well!

@leotrs
Copy link
Collaborator

leotrs commented Jan 20, 2022

To move this forward we need a consensus on the name. Current alternatives are:

  1. Rename the current function (returns all neighbors in a single list) to graph_neighbors and the new one would be neighbors.

  2. Current function remains as neighbors and new one is called egonet.

  3. Something else.

Please vote!

@iaciac
Copy link
Collaborator Author

iaciac commented Jan 20, 2022

Obviously, let's start a third option :D

  1. Keep this exact function with the name neighbor_nodes. Then, to get what I was proposing one could simply use the membership function discussed in Interface of NodeView #40 and from there it would be straightforward.

What do you think?

@maximelucas
Copy link
Collaborator

Our reasoning to call it neighbor_nodes instead of neighbor is to make it explicit that it returns node IDs (and not edge IDs).

@leotrs
Copy link
Collaborator

leotrs commented Jan 20, 2022

In that case I would also suggest a neighbor_edges then.

Also, I think returning the edges a node is part of (its membership) vs returning the edges it is a part of but with that node removed are sufficiently different that they could be different methods. So this would leave suggestion number 3 as implementing neighbor_nodes and neighbor_edges, both returning lists of ids, and membership and egonet, returning lists of edges.

....which perhaps seems like overkill? Not sure

@leotrs
Copy link
Collaborator

leotrs commented Jan 20, 2022

Perhaps a good compromise is to implement only two functions: members and membership and have them take one parameter, ids, which controls whether they return a list of ids or a list of objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improve Make an existing feature better
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants