[PartitionedGraphs] Small changes #114
Conversation
- `vertices(::QuotientView)` now directly returns the keys `paritioned_vertices` return value. - `edges(::QuotientView)` now directly returns the edges of the `quotient_graph` return value (to coincide with interface overloading priority) - Converting a `QuotientView` to graph now calls `quotient_graph` directly - Adding methods for return directed/undirected graph types. Fix imports Fix rebase
This behaves similarly to `partitionedgraph` function.
…ype by default This function can now be used `SimpleGraph` etc without promoting to `NamedGraph`.
This function constructs a graph with no edges, but with vertices of the quotient graph.
… depending on graph type.
…eating quotient graph
…ype as a parameter Allows for more generic quotient graph types.
…s for `PartitionedGraph`
…methods for `AbstractSimpleGraph`.
- argument `vertices` must be of type `Base.OneTo{Int}`.
…e, and triple argument methods. This interface is to overload `similar_graph(graph)`, i.e. the single argument method.
This function now acts similarly to `similar_graph`.
| function add_vertices!(graph::AbstractSimpleGraph, vertices::Base.OneTo{Int}) | ||
| for _ in vertices | ||
| add_vertex!(graph) | ||
| end | ||
| return graph | ||
| end |
There was a problem hiding this comment.
Where is this used in practice?
There's an awkward design issue when trying to write code that is generic between SimpleGraphs and NamedGraphs, since as you know SimpleGraphs are one-based. For NamedGraphs, add_vertices!(g, vs) has the property that the resulting graph has vertices union(vertices(g), vs), which isn't the case for how you've defined this function above.
I wonder if we can get away with just not defining this function for now, Graphs.jl already has an add_vertices! function for SimpleGraphs: https://juliagraphs.org/Graphs.jl/dev/core_functions/core/#Graphs.add_vertices!-Tuple%7BAbstractGraph,%20Integer%7D. I think my goal with GraphsExtensions was to make code that is really generic between SimpleGraphs and NamedGraphs, in the sense that if I input a SimpleGraph or a NamedGraph with the same integer vertices and other arguments, the behavior is the same.
There was a problem hiding this comment.
Another idea could be to define this as:
function add_vertices!(g::AbstractSimpleGraph, vs::AbstractUnitRange)
first(vs) >= 1 || throw(ArgumentError("Vertices must be greater than or equal to 1 for AbstractSimpleGraph"))
nvs = length(setdiff(vs, vertices(g)))
Graphs.add_vertices!(g, nvs)
return g
endwhich would have the property that the resulting graph has vertices union(vertices(g), vs). But that may be a little bit tricky so maybe we should only define that if we have a compelling use case.
There was a problem hiding this comment.
I'll check if this method is even needed.
| # So partitions of `AbstractSimpleGraph` make `AbstractSimpleGraph`s | ||
| function _quotient_graph_from_vertices(graph::PartitionedView, vertices) | ||
| return _quotient_graph_from_vertices(unpartitioned_graph(graph), vertices) | ||
| end |
There was a problem hiding this comment.
Should this be similar_quotient_graph?
|
|
||
| edge_list = edges(g) | ||
|
|
||
| for edge in edge_list | ||
| rem_edge!(g, edge) | ||
| add_edge!(g, reverse(edge)) | ||
| end |
There was a problem hiding this comment.
This was something that I only just appreciated when reading this function, but in general the edge list can change if the graph is changed:
julia> using Graphs
julia> g = path_graph(5)
{5, 4} undirected simple Int64 graph
julia> e = edges(g)
SimpleEdgeIter 4
julia> collect(e)
4-element Vector{Graphs.SimpleGraphs.SimpleEdge{Int64}}:
Edge 1 => 2
Edge 2 => 3
Edge 3 => 4
Edge 4 => 5
julia> rem_edge!(g, 2, 3)
true
julia> e
SimpleEdgeIter 3
julia> collect(e)
3-element Vector{Graphs.SimpleGraphs.SimpleEdge{Int64}}:
Edge 1 => 2
Edge 3 => 4
Edge 4 => 5
Which is documented here: https://juliagraphs.org/Graphs.jl/dev/core_functions/interface/#Graphs.edges-Tuple%7BAny%7D. I'm not sure if that causes problems for this implementation, maybe to be safe we should collect the edges. I think maybe for NamedGraph, edges(::NamedGraph) isn't a view of the edges, but probably it should be both for efficiency and to match the design of Graphs.jl.
There was a problem hiding this comment.
Note that the same thing is true of vertices(g), i.e. it is supposed to be a view into the vertices of g (https://juliagraphs.org/Graphs.jl/dev/core_functions/interface/#Graphs.vertices-Tuple{AbstractGraph}). Again, I think maybe it isn't implemented that way right now for NamedGraph but probably it should be implemented that way.
Co-authored-by: Matt Fishman <mtfishman@users.noreply.github.com>
…textype`; add explict `similar_graph` method for vertex conversion
2 participants
This PR includes the following changes to [PartitionedGraphs]
NamedGraphsinterface functions to various data types.QuotientViewnow directly callsquotient_graph.edgesandverticesonQuotientViewnow returns according to the interface priority.QuotientViewsofAbstractSimpleGraphswill now return a graph of similar type.