[PartitionedGraphs] Small changes #114

jack-dunham · 2025-11-24T22:32:36Z

This PR includes the following changes to [PartitionedGraphs]

Added some additional NamedGraphs interface functions to various data types.
Constructing a graph from a QuotientView now directly calls quotient_graph.
Calling edges and vertices on QuotientView now returns according to the interface priority.
Constructing graphs from QuotientViews of AbstractSimpleGraphs will now return a graph of similar type.

- `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

…rtitionedGraph`

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`.

src/lib/GraphsExtensions/src/abstractgraph.jl

src/lib/GraphsExtensions/src/simplegraph.jl

src/lib/PartitionedGraphs/src/partitionedview.jl

test/test_namedgraph.jl

src/abstractnamedgraph.jl

Co-authored-by: Matt Fishman <[email protected]>

The function `to_graph_indexing` now dispatches to `to_graph_indices` for type of `AbstractGraphIndices` and `to_graph_index` otherwise.

…stic about the iterables they take Previous behaviour was to accept `Vertices`/`Edges` only.

…sed on `eltype` of `vertices` argument, if provided.

jack-dunham · 2025-12-15T17:04:32Z

Consider if similar_graph should allow converting to the eltype of the input vertices.

On this point, somewhere this could be problematic is if vertices is a vector literal resulting in a vector of eltype differing to the vertextype of the graph, or the vertices argument has some abstract eltype due to some type instability somewhere. For example:

In [19]: g = NamedGraph([1.0, 1.5, 2.0])
NamedGraph{Float64} with 3 vertices:
3-element NamedGraphs.OrderedDictionaries.OrderedIndices{Float64}:
 1.0
 1.5
 2.0

and 0 edge(s):
NamedEdge{Float64}[]

In [20]: similar_graph(g, [1, 2]) # use integer literals out of laziness
NamedGraph{Int64} with 2 vertices:
2-element NamedGraphs.OrderedDictionaries.OrderedIndices{Int64}:
 1
 2

and 0 edge(s):
NamedEdge{Int64}[]

In [21]: similar_graph(g, Any[1, 1.5]) # accidentally `eltype==any` due to type instability somewhere
NamedGraph{Any} with 2 vertices:
2-element NamedGraphs.OrderedDictionaries.OrderedIndices{Any}:
 1
 1.5

and 0 edge(s):
NamedEdge{Any}[]

One ends up with graphs with vertextype that might not be intended. However perhaps it is sensible for similar_graph to not be so opinionated here, and have it up to the user to either be careful about the eltype of vertices or be explicit by manually calling:

h = similar_graph(g)
add_vertices!(h, ...)

to make sure vertextype(g) == vertextype(h).

I have made changes to similar_graph to align with this new, less opinionated behavior, however just wondering @mtfishman if you have a perspective in light of this example.

mtfishman · 2025-12-15T17:26:41Z

It's a good point, I was wondering about that as well. I would lean towards taking the input literally for now and leave it up to users to input vertices with the intended element/vertex type, and we could circle back to that if it seems to cause problems.

…nt vertices and edges.

mtfishman · 2025-12-18T17:50:47Z

@jack-dunham I had forgetten, but Base.to_index handles both scalar indexes and array of indices: https://github.com/JuliaLang/julia/blob/v1.12.3/base/indices.jl#L279-L315 (Base.to_indices refers to multiple dimensions). So maybe we could just go with to_graph_index for both single vertices/edges and sets of vertices/edges.

…bject when iterating.

…ions.

…ype constructor for `QuotientVertices`.

mtfishman · 2025-12-18T22:31:12Z

src/lib/PartitionedGraphs/src/quotientvertex.jl

+    end
+end
+
+quotients(qvs::QuotientVertexVertices) = getfield(qvs, :quotients)


I think this is a case where it would be better to not define this function and instead directly use qvs.quotients, to make it clearer this is just accessing data in the struct and not really a generic function.

mtfishman · 2025-12-19T22:04:44Z

src/lib/PartitionedGraphs/src/quotientedge.jl

+    function QuotientEdges{V, E}(edges::Es) where {V, E, Es}
+        @assert E <: eltype(Es)
+        return new{V, E, Es}(edges)
+    end


Related to offline discussions, I think we should remove this constructor and just get the edge type from the input (we can revisit if use cases arise that don't have reasonable alternatives).

mtfishman · 2025-12-19T23:04:26Z

src/lib/PartitionedGraphs/src/quotientedge.jl

+function Base.iterate(qes::QuotientEdges, state = nothing)
+    if isnothing(state)
+        out = iterate(qes.edges)
+    else
+        out = iterate(qes.edges, state)
+    end
+    if isnothing(out)
+        return nothing
+    else
+        (v, s) = out
+        return (QuotientEdge(v), s)
+    end
+end


The iterate definitions seem a bit complicated to me. It looks like they are all basically a lazy map over iterating over one of the fields. It seems like that can be implemented more simply like this:

struct Wrapper{T} data::T end Base.length(w::Wrapper) = length(w.data) Base.eltype(w::Wrapper) = String # Maybe use `Base.promote_op` if the map is more complicated? function Base.iterate(w::Wrapper, state...) return iterate(Iterators.map(string, w.data), state...) end

which enables:

julia> x = Wrapper([1, 2, 3]) Wrapper{Vector{Int64}}([1, 2, 3]) julia> collect(x) 3-element Vector{String}: "1" "2" "3"

I think then we wouldn't need iterate_graph_indices.

mtfishman · 2025-12-19T23:05:35Z

src/lib/PartitionedGraphs/src/quotientedge.jl

+    end
+end
+
+quotients(qes::QuotientEdgeEdges) = getfield(qes, :quotients)


Is this function needed? Could we just use direct field access?

mtfishman · 2025-12-19T23:11:37Z

src/abstractgraphindices.jl

+    Vertices(vertices::Vs) where {Vs} = new{eltype(Vs), Vs}(vertices)
+end
+
+Vertices(v1, v2, vertices...) = Vertices(vcat([v1, v2], collect(vertices)))


I'm not a fan of this constructor, I think we should keep constructors simpler and stricter, for example just have one kind of expected input (in this case, only accept a single collection of vertices). Wouldn't this kind of use case already be handled by the basic constructor, and you can pass a Tuple of vertices, i.e. Vertices(("a", "b", "c"))?

As a concrete issue with this constructor, it makes Vertices(["a"]) ambiguous, since it isn't clear if that is supposed to be the set of vertices, or just the length-1 case of passing multiple arguments (which then if I understand this constructor, would interpret ["a"] as a vertex itself). So it is better to just avoid that kind of ambiguity and keep constructors stricter.

mtfishman · 2025-12-19T23:11:46Z

src/abstractgraphindices.jl

+    end
+end
+
+Edges(e1, e2, edges...) = Edges(vcat([e1, e2], collect(edges)))


Same comment as above.

mtfishman · 2025-12-19T23:12:51Z

src/abstractgraphindices.jl

+Base.iterate(gi::AbstractGraphIndices, state = nothing) = iterate_graph_indices(identity, gi, state)
+function iterate_graph_indices(f, gi::AbstractGraphIndices, state)
+    if isnothing(state)
+        out = iterate(parent_graph_indices(gi))
+    else
+        out = iterate(parent_graph_indices(gi), state)
+    end
+    if isnothing(out)
+        return nothing
+    else
+        (v, s) = out
+        return (f(v), s)
+    end
+end


Same comment as above regarding a simpler iterate definition.

mtfishman · 2025-12-19T23:14:05Z

src/lib/PartitionedGraphs/src/quotientvertex.jl

+function Base.iterate(qvs::QuotientVertices, state = nothing)
+    return NamedGraphs.iterate_graph_indices(QuotientVertex, qvs, state)
+end


Same comment as above regarding a simpler iterate definition.

mtfishman · 2025-12-19T23:14:31Z

src/lib/PartitionedGraphs/src/quotientvertex.jl

+function Base.iterate(qvs::QuotientVertexVertices, state = nothing)
+    return NamedGraphs.iterate_graph_indices(v -> quotient_index(qvs)[v], qvs, state)
+end


Same comment as above regarding a simpler iterate definition.

mtfishman · 2025-12-19T23:16:41Z

src/lib/PartitionedGraphs/src/quotientvertex.jl

+    end
+end
+
+QuotientVertices(vertices...) = QuotientVertices(collect(vertices))


Same comment as the ones for the analogous Vertices and Edges constructors.

Jack Dunham added 24 commits November 24, 2025 16:44

[PartitionedGraphs] Add directed/undirected graph type methos for `Pa…

be7d219

…rtitionedGraph`

[PartitionedGraphs] Add similar_type method for QuotientView.

e453d7f

[PartitionedGraphs] Add quotientview interface function

9487b51

This behaves similarly to `partitionedgraph` function.

[PartitionedGraphs] quotient_graph now returns a graph of similar t…

6bfb9b4

…ype by default This function can now be used `SimpleGraph` etc without promoting to `NamedGraph`.

Version bump

a7d5a58

[PartitionGraphs] fallback method for show

b6f1f6b

Fix missing import

3702ef6

Remove broken import

4dc4611

[PartitionedGraphs] Add edgeless_quotient_graph interface function.

f1641a2

This function constructs a graph with no edges, but with vertices of the quotient graph.

[PartitionedGraphs] quotient_graph now returns a (un)directed graph…

36381cf

… depending on graph type.

[PartitionedGraphs] Explictly construct edges of correct type when cr…

703363a

…eating quotient graph

[PartitionedGraphs] PartitionedGraph now takes the quotient graph t…

d277466

…ype as a parameter Allows for more generic quotient graph types.

Fix typo as graph_type -> graph.

e55cb9f

[PartitionedGraphs] Add directed/undireced graph constructor function…

fa2608c

…s for `PartitionedGraph`

Remove redundant directed_graph method, add fallbacks NamedGraph …

e81da68

…methods for `AbstractSimpleGraph`.

[PartitionedGraphs] Remove overlapping method

d7fe1ee

Add fall back for position_graph_type based on promote_op.

809f5b8

Test structs now implemented the correct interface for NamedGraphs

02789eb

Add add_vertices! method for AbstractSimpleGraph;

0ae1425

- argument `vertices` must be of type `Base.OneTo{Int}`.

Rename graph_from_vertices -> similar_graph and add single, doubl…

e1627a9

…e, and triple argument methods. This interface is to overload `similar_graph(graph)`, i.e. the single argument method.

Rename edgeless_quotient_graph to similar_quotient_graph.

1929506

This function now acts similarly to `similar_graph`.

Add tests for similar_graph function

3cd86a3

Add specific and fallback methods for Base.reverse with tests.

4ae23ee

mtfishman reviewed Dec 8, 2025

View reviewed changes

src/lib/GraphsExtensions/src/abstractgraph.jl Outdated Show resolved Hide resolved

mtfishman reviewed Dec 9, 2025

View reviewed changes

src/lib/GraphsExtensions/src/simplegraph.jl Outdated Show resolved Hide resolved

mtfishman reviewed Dec 9, 2025

View reviewed changes

src/lib/PartitionedGraphs/src/partitionedview.jl Outdated Show resolved Hide resolved

mtfishman reviewed Dec 9, 2025

View reviewed changes

test/test_namedgraph.jl Outdated Show resolved Hide resolved

mtfishman reviewed Dec 9, 2025

View reviewed changes

src/abstractnamedgraph.jl Show resolved Hide resolved

Remove left over @warn statement in test file.

d6d98fa

Co-authored-by: Matt Fishman <[email protected]>

Jack Dunham added 4 commits December 15, 2025 11:29

Rename top level function to_graph_indexing from to_graph_indices

625715c

The function `to_graph_indexing` now dispatches to `to_graph_indices` for type of `AbstractGraphIndices` and `to_graph_index` otherwise.

Add basic tests for graph indexing.

8093c27

Structs QuotientVertexVertices and QuotientEdgeEdges are now agno…

a6ef154

…stic about the iterables they take Previous behaviour was to accept `Vertices`/`Edges` only.

Function similar_graph now converts the vertex type of the graph ba…

2cb5d06

…sed on `eltype` of `vertices` argument, if provided.

Jack Dunham added 6 commits December 15, 2025 18:01

No longer collect vertices/edges when converting quotients

3a7591c

Basic graph indexing tests for partitioned graphs

25f524e

Fix incorrect name in error messages

f8f38e4

Add QuotientVertices/Edges structs as wrappers around lists of quotie…

caa1772

…nt vertices and edges.

Deprec AbstractGraphIndices indexing

c5bcfe5

Overhaul edge/vertex collection handling.

4e15989

Jack Dunham added 5 commits December 18, 2025 13:19

QuotientEdges/Vertices now returns a QuotientEdge or QuotientVertex o…

9ae8f4f

…bject when iterating.

Use to_graph_index instead of to_graph_indexing and related funct…

5b977c9

…ions.

Fix convert_vertextype methods for AbstractNamedGraph.

f569a9a

Remove Sub from QuotientXSubX struct names; remove explict vertex t…

20c385c

…ype constructor for `QuotientVertices`.

Cosmetic changes

a4a1ad0

mtfishman reviewed Dec 18, 2025

View reviewed changes

Jack Dunham added 2 commits December 19, 2025 16:36

Expand on vertex indexing.

2138b82

Allow subgraphing w.r.t QuotientVertexVertex

453f352

mtfishman reviewed Dec 19, 2025

View reviewed changes

src/abstractgraphindices.jl

end

end

Edges(e1, e2, edges...) = Edges(vcat([e1, e2], collect(edges)))

Copy link

Member

mtfishman Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above.

mtfishman reviewed Dec 19, 2025

View reviewed changes

[PartitionedGraphs] Small changes #114

Are you sure you want to change the base?

[PartitionedGraphs] Small changes #114

Uh oh!

Conversation

jack-dunham commented Nov 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jack-dunham commented Dec 15, 2025

Uh oh!

mtfishman commented Dec 15, 2025

Uh oh!

mtfishman commented Dec 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtfishman Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mtfishman Dec 19, 2025 •

edited

Loading