Better functionality for PartitionedGraph

[JoeyT1994]

This PR adds improved functionality and function naming for the PartitionedGraph type.

Firstly, we add support for reverse(pe::PartitionedEdge) which previously required the user to unwrap and wrap the edge again.

Secondly, there is proper support for getting edges out of a PartitionedGraph.
New functions in this domain:

• partition_edge(pg, edge) is renamed which_partitionedge(pg, edge::AbstractEdge) to be more consistent with the naming of the vertex version which_partition(pg, vertex).
• which_partitionedges(pg, edges) and which_partitions(pg, vertices) have been added to support the vectorized version of those functions
• partition_edges(pg, pe::PartitionEdge) is renamed edges(pg, pe::PartitionEdge) (and a vectorized version is added) to be more clear about the return type
• partitionedges(pg) and partitionvertices(pg) is added to just return the lists of edges and vertices of the partitioned_graph(pg) (wrapped in the PartitionEdge and PartitionVertex type)

These functions are exported.

[mtfishman]

Thanks.

I prefer partition_edge/partition_edges over which_partitionedge/which_partitionedges and partition_vertex/partition_vertices over which_partition/which_partitions.

I agree with renaming partition_edges to just edges.

I would call them partition_edges(pg)/partition_vertices(pg) instead of partitionedges(pg)/partitionvertices(pg), since we generally are using snake casing for other functions with similar names in this package.

[JoeyT1994]

Yeah I am happy with that naming. The only issue is that we also have partition_vertices(pg; kwargs...) defined for when calling a partitioning backend like KaHyPar. Any thoughts on what we should rename that function to avoid the clash?

Maybe we could just call it partition(pg; partitioning_kwargs...) ?

[mtfishman]

I see, yeah that is an unfortunate name clash. I don't like partition(pg) for that since it doesn't indicate that it is outputting sets/groups of vertices (I would think that it might be outputting a partitioned graph, and in fact we might want to make that a simpler API for constructing a PartitionedGraph).

The problem seems to basically be that when reading partition_vertices(...), partition could be interpreted either as a verb (as in, the function is performing a partitioning of the vertices), or as a noun (as in, the vertices of a partitioned graph). It feels more natural to me to interpret it as a verb, and I think for that reason the names PartitionVertex and PartitionEdge have felt a bit awkward to me, but I can't think of better names for those so we are left with this conundrum.

One way out could be to call the function that partitions the vertices with KaHyPar/Metis partitioned_vertices(pg; kwargs...), to name it after what it is outputting rather than what it is doing (as in, that function is outputting sets/groups of partitioned vertices).

[JoeyT1994]

Okay will go with partitioned_vertices(pg; kwargs...) for now

[codecov-commenter]

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (40f0edd) 78.83% compared to head (941b4d2) 77.70%.

Files	Patch %	Lines
src/Graphs/partitionedgraphs/partitionedgraph.jl	20.00%	12 Missing ⚠️
...aphs/partitionedgraphs/abstractpartitionedgraph.jl	22.22%	7 Missing ⚠️
src/Graphs/partitionedgraphs/partitionedge.jl	0.00%	2 Missing ⚠️
.../Graphs/partitionedgraphs/abstractpartitionedge.jl	0.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #45      +/-   ##
==========================================
- Coverage   78.83%   77.70%   -1.14%     
==========================================
  Files          31       31              
  Lines        1167     1184      +17     
==========================================
  Hits          920      920              
- Misses        247      264      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mtfishman]

Looks good, thanks. Could you add tests for the new functions?

[mtfishman]

Something I realized is that I think the snake casing in partition_vertex/partition_vertices and partition_edge/partition_edges may not be the right way to go. My thinking is that, with the current design, we are establishing a convention that partition_vertex(...) will output a PartitionVertex, partition_vertices will output a list of PartitionVertex, etc., which I think is a good convention to establish and makes the code clearer (and makes it clearer to us what to name functions going forward).

I think removing the snake casing, i.e. changing to partitionvertex, partitionvertices, etc., could make that convention even more obvious. To me, reading those new names makes it clearer that there is a connection between the name of the function and the type of object it is outputting (i.e., it makes it clearer that "partition" in the function name partitionvertex is a noun rather than a verb, and will output an object of type PartitionVertex).

I'm on the fence about that since we use snake casing in other functions, let me know what you think.

[JoeyT1994]

I think partitionvertex (as opposed to partition_vertex ) is clearer to me in terms of what it is outputting (i.e. a PartitionVertex) and so I am in favor of that to avoid ambiguity about partition being a verb here. In fact here we could argue that we are not even necessarily forgoing snake casing as we have defined a PartitionVertex type and so partitionvertex is just simply a one word noun and no spaces are required?

[mtfishman]

Yeah, I think that is a more succinct reason for using the style partitionvertex. Also, it is common practice in Julia to have two sets of constructors for types, one using camel casing and one using lower casing (for example Tuple/tuple and String/string) which have different behaviors. Often the camel cased constructor is lower level and stricter. So this is a good example of following that convention.

[JoeyT1994]

Okay great. I have gone with that naming convention for now! I also added further testing.

[mtfishman]

Looks good, thanks!