MetaGraphsNext.jl icon indicating copy to clipboard operation
MetaGraphsNext.jl copied to clipboard

Different code- vs. label- API for `has_edge` vs. `add_edge!`

Open thchr opened this issue 1 year ago • 6 comments

Example below:

g = SimpleGraph()
mg = MetaGraph(g, String, Nothing, Nothing)
for c in 'a':'d'
    add_vertex!(mg, string(c))
end
add_edge!(mg, "a", "b")
has_edge(mg, "a", "b")
ERROR: method has_edge not implemented.

I.e., add_edge! works on the labels of the MetaGraph but has_edge works with the codes. This is especially surprising if you (contrary to recommendations) go with Integer labels:

g = SimpleGraph()
mg = MetaGraph(g, Int, Nothing, Nothing) # <--- Int label
for i in 3:6
    add_vertex!(mg, i)
end
add_edge!(mg, 3, 4)
has_edge(mg, 3, 4)

(where, current API requires us to do has_edge(mg, code_for(mg, 3), code_for(mg, 4)) to get a meaningful result.

thchr avatar Mar 25 '24 12:03 thchr

You're right, a function has_edge(mg, label1, label2) would be warranted. But beware that since has_edge(mg, code1::Integer, code2::Integer) has to exist, if you're using integers as labels, not sure which of the two methods would get picked.

gdalle avatar Mar 25 '24 12:03 gdalle

Yeah, that's a potential issue indeed: but should there really be a code-access has_edge for a MetaGraph? It seems much more natural to require that one does has_edge(mg.graph, code1, code2) if one wants to query the underlying graph? Or, is this to comply with a Graphs.jl interface?

Alternatively, has there been any discussion of introducing a special Label or Code type? In the latter case, it could simply wrap an integer.

thchr avatar Mar 25 '24 16:03 thchr

It is necessary to comply with the Graphs.jl interface, otherwise you'd have to call all graph algorithms with algorithm(mg.graph, code)

gdalle avatar Mar 25 '24 16:03 gdalle

I see. That's too bad.

Is there a sense in which some of these issues could be alleviated if Graphs.jl had an internal type like

struct Vertex{T<:Integer}
   v :: T
end

with the iterates of vertices(g) then being of type Vertex? This would be symmetric with Edge{T} and edges(g) and seems it would fix many of these issues. I suppose it might be breaking - but is it part of the consideration for Graphs 2.0?

thchr avatar Mar 26 '24 08:03 thchr

One of the key drivers behind Graphs.jl 2.0 conversations is the desire to support arbitrary vertex types. In that sense, we will no longer need the distinction between labels and vertices, at least on a surface level

gdalle avatar Mar 26 '24 10:03 gdalle

As of right now I don't think there is a good solution to your problem, aside from documenting this caveat. Do you want to open a PR? (I haven't forgotten the simple paths one, no worries)

gdalle avatar Mar 27 '24 08:03 gdalle