Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mycielski operator #177

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Graphs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ complement, reverse, reverse!, blockdiag, union, intersect,
difference, symmetric_difference,
join, tensor_product, cartesian_product, crosspath,
induced_subgraph, egonet, merge_vertices!, merge_vertices,
mycielski,

# bfs
gdistances, gdistances!, bfs_tree, bfs_parents, has_path,
Expand Down
71 changes: 71 additions & 0 deletions src/operators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -851,3 +851,74 @@ function merge_vertices!(g::Graph{T}, vs::Vector{U} where U <: Integer) where T

return new_vertex_ids
end

"""
mycielski(g)

Returns a graph after applying the Mycielski operator to the input. The Mycielski operator
returns a new graph with `2n+1` vertices and `3e+n` edges and will increase the chromatic
Copy link
Contributor

Choose a reason for hiding this comment

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

I think using the symbol e for the number of edges is a bit unusual, when we use n for the number of vertices at the same time. Maybe there is a better way?

number of the graph by 1.

The Mycielski operation can be repeated by using the `iterations` kwarg. Each time, it will
apply the operator to the previous iterations graph.

For each vertex in the original graph, that vertex and a copy of it are added to the new graph.
Then, for each edge `(x, y)` in the original graph, the edges `(x, y)`, `(x', y)`, and `(x, y')`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, the usual convention for variables that represent vertices are the symbols u, v and w or s and t if we talk about some path finding algorithm.

are added to the new graph, where `x'` and `y'` are the "copies" of `x` and `y`, respectively.
In other words, the original graph is present as a subgraph, and each vertex in the original graph
is connected to all of its neighbors' copies. Finally, one last vertex `w` is added to the graph
and an edge connecting each copy vertex `x'` to `w` is added.

See [Mycielskian](https://en.wikipedia.org/wiki/Mycielskian) for more information.

# Examples
```jldoctest
julia> c = CycleGraph(5)
{5, 5} undirected simple Int64 graph

julia> m = Graphs.mycielski(c)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
julia> m = Graphs.mycielski(c)
julia> gm = Graphs.mycielski(g)

We usually use m to represent some kind of number such as the number of vertices or the number of edges and use symbols such as g and h for graphs.

{11, 20} undirected simple Int64 graph

julia> collect(edges(m))
20-element Vector{Graphs.SimpleGraphs.SimpleEdge{Int64}}:
Edge 1 => 2
Edge 1 => 5
Edge 1 => 7
Edge 1 => 10
Edge 2 => 3
Edge 2 => 6
Edge 2 => 8
Edge 3 => 4
Edge 3 => 7
Edge 3 => 9
Edge 4 => 5
Edge 4 => 8
Edge 4 => 10
Edge 5 => 6
Edge 5 => 9
Edge 6 => 11
Edge 7 => 11
Edge 8 => 11
Edge 9 => 11
Edge 10 => 11
```
"""
@traitfn function mycielski(g::AbstractGraph::(!IsDirected); iterations = 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@traitfn function mycielski(g::AbstractGraph::(!IsDirected); iterations = 1)
@traitfn function mycielski(g::SimpleGraph; iterations::Int = 1)

The problem we have at the moment, that for graphs where we have some kind of edge weights/data, it is not clear what we mean by adding an edge. This is definitely something we should sort out in the future, but in the meantime I would suggest to restrict this function to SimpleGraphs.

Also, would suggest to restrict the type of iterations to to Int or Integer.

out = deepcopy(g)
Copy link
Member

Choose a reason for hiding this comment

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

I think that deepcopy(g) could slow down and uses a lot of memory especially for big graphs.

Copy link
Contributor Author

@mcognetta mcognetta Oct 5, 2022

Choose a reason for hiding this comment

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

These operators are not supposed to mutate the input right? I see some have ! variants, but not all. Perhaps there should be a mycielski! and mycielski(g; iterations) = mycielski!(deepcopy(g); iterations=iterations) method. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Yes I agree with you. I was only thinking about the case in which we can mutate the input.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also support having both mycielski! and mycielski. We currently have something similar for transitiveclosure and transitiveclosure! at the moment.

for _ in 1:iterations
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for _ in 1:iterations
for _ in Base.OneTo(iterations)

To be honest though, I am not sure if that makes a big difference for the case where iterations is not of type Int.

N = nv(out)
add_vertices!(out, N + 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

This might fail, if the graph eltype does not support adding that many vertices. This might be especially bad, if we also create a mutating mycielski! function, as then this function might fail and leave g in some in-between state. So we probably should check at the start of the function if we have enough capacity to add that many vertices.

For the non-mutating mycielski function, we might also allow one to provide another larger eltype as an optional argument, such that we can retu

w = nv(out)
for e in collect(edges(out))
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we allocate an new vector for all edges, this would need quite a lot of space. Of course it is not possible to just iterate over the edges without collecting them, but maybe we can iterate of the first N vertices and allocate just a vector of the outneighbors of each vertex?

x=e.src
y=e.dst
add_edge!(out, x, y+N)
add_edge!(out, x+N, y)
end

for v in 1:N
add_edge!(out, v+N, w)
end
end
return out
end
24 changes: 24 additions & 0 deletions test/operators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -320,4 +320,28 @@
@testset "Length: $g" for g in testgraphs(SimpleGraph(100))
@test length(g) == 10000
end


@testset "Mycielski Operator" begin
g = complete_graph(2)

m = mycielski(g; iterations = 8)
@test nv(m) == 767
@test ne(m) == 22196

# ensure it is not done in-place
@test nv(g) == 2
@test ne(g) == 1

# check that mycielski preserves triangle-freeness
g = complete_bipartite_graph(10, 5)
m = mycielski(g)
@test nv(m) == 2*15 + 1
@test ne(m) == 3*50 + 15
@test all(iszero, triangles(m))

# ensure it is not done in-place
@test nv(g) == 15
@test ne(g) == 50
end
Copy link
Contributor

Choose a reason for hiding this comment

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

These tests are good, but I think we need to cover some additional test cases such as:

  • tests for graphs with self-loops
  • tests for graphs with zero vertices
  • tests for graphs with isolated vertices
  • tests for graphs with different eltypes - the testgraphs utility function can help here

end