Skip to content

Conversation

@mofeing
Copy link
Member

@mofeing mofeing commented Jul 5, 2025

To do

  •  Add methods for neighbors from incoming or outgoing edges
  •  Decide about directed hypergraphs

@mofeing mofeing linked an issue Jul 5, 2025 that may be closed by this pull request
@mofeing
Copy link
Member Author

mofeing commented Jul 5, 2025

cc @gdalle

@gdalle
Copy link

gdalle commented Jul 9, 2025

This is looking good, but I think we have a naming problem. To be consistent, I would suggest that the function name refers to the type of stuff it returns, as opposed to the type of stuff it ingests (which the user already knows because the arguments are given). Here's my suggestion:

current proposed signature
edge_incidents(g, e) incident_vertices(g, e) edge -> vertices
vertex_src(g, e) source_vertex(g, e) edge -> vertex
vertex_dst(g, e) destination_vertex(g, e) edge -> vertex
vertex_incidents(g, v) incident_edges(g, v) vertex -> edges
edges_in(g, v) incoming_edges(g, v) vertex -> edges
edges_out(g, v) outgoing_edges(g, v) vertex -> edges
vertex_neighbors(g, v) neighbor_vertices(g, v) vertex -> vertices
- successor_vertices(g, v) vertex -> vertices
- predecessor_vertices(g, v) vertex -> vertices
edge_neighbors(g, e) neighbor_edges(g, e) edge -> edges

In terms of verbosity, the choice of Graphs.jl to go with very short names (src, dst, nv, ne) is more of an obstacle to readability than anything in my opinion.

For comparison, here are the access APIs from networkx:

@mofeing
Copy link
Member Author

mofeing commented Jul 15, 2025

This is looking good, but I think we have a naming problem. To be consistent, I would suggest that the function name refers to the type of stuff it returns, as opposed to the type of stuff it ingests (which the user already knows because the arguments are given). Here's my suggestion:

I thought a lot about it and was doubting about what I chose and what you suggest. Now that you bring back this topic, I think is better what you suggest.

In terms of verbosity, the choice of Graphs.jl to go with very short names (src, dst, nv, ne) is more of an obstacle to readability than anything in my opinion.

I agree, that's why I went with nvertices and nedges.

@mofeing
Copy link
Member Author

mofeing commented Jul 15, 2025

about successor_vertices(g, v), predecessor_vertices(g, v), i'm okay with adding them but not sure if as part of the interface. i'm trying to keep the interface small and self-containable, and they can be just used as:

successor_vertices(g, v) = destination_vertex.((g,), outgoing_edges(g, v))
predecessor_vertices(g, v) = source_vertex.((g,), incoming_edges(g, v))

i.e. in principle, we don't need them to be specialized by implementors (or can't see when). maybe i'm wrong?

@gdalle
Copy link

gdalle commented Jul 15, 2025

The functions that we don't need in the interface are functions that can be implemented through other functions without a slowdown. Take SimpleWeightedDiGraph for instance.

  • How would you implement outgoing_edges?
  • How would you deduce outgoing_neighbors?
  • Is it as fast as the original implementation, which is just a view of an integer array?

Besides, you're still looking at things with your hypergraph, edge-centric mindset. Most people I know won't give a shit about edges, they want neighbors (that's why Graphs.jl was designed this way in the first place). I understand the need to support edges too, but it shouldn't be done in a way that is detrimental to neighbor efficiency.

@mofeing
Copy link
Member Author

mofeing commented Jul 16, 2025

Okay, I've added predecesor_vertices and successor_vertices to the directed network interface.

i think the next thing we should revisit are the mutating methods, since here there is kind of a clear distinction between adjacent and incident matrix base implementations, but i will do it in another issue / pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Directed graphs?

3 participants