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 regular tree generator #197

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 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/SimpleGraphs/SimpleGraphs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export AbstractSimpleGraph,
cycle_digraph,
binary_tree,
double_binary_tree,
regular_tree,
roach_graph,
clique_graph,
barbell_graph,
Expand Down
54 changes: 54 additions & 0 deletions src/SimpleGraphs/generators/staticgraphs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,60 @@ function double_binary_tree(k::Integer)
return g
end

"""
regular_tree([T=Int64, ], k::Integer, z::integer)
Copy link
Member

Choose a reason for hiding this comment

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

Except for binary_tree and double_binary_tree all the other functions of this file have the following style:

Suggested change
regular_tree([T=Int64, ], k::Integer, z::integer)
regular_tree(T, k, z)


Create a regular tree or [perfect z-ary tree](https://en.wikipedia.org/wiki/M-ary_tree#Types_of_m-ary_trees):
a `k`-level tree where all nodes except the leaves have exactly `z` children.
For `z = 2` one recovers a binary tree.

# Examples
```jldoctest
julia> regular_tree(4, 3)
{40, 39} undirected simple Int64 graph

Comment on lines +549 to +551
Copy link
Member

Choose a reason for hiding this comment

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

I would remove it and I would put it in the docstring of the function regular_tree(k,z).

Suggested change
julia> regular_tree(4, 3)
{40, 39} undirected simple Int64 graph

julia> regular_tree(Int8, 3, 2)
{7, 6} undirected simple Int8 graph

julia> regular_tree(5, 2) == binary_tree(5)
true
Comment on lines +554 to +556
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

Suggested change
julia> regular_tree(5, 2) == binary_tree(5)
true

```
"""
function regular_tree(T::Type{<:Integer}, k::Integer, z::Integer)
z <= 0 && throw(DomainError(z, "number of children must be positive"))
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we also check for k >= 0 here?

z == 1 && return path_graph(T(k))
k <= 0 && return SimpleGraph(zero(T))
k == 1 && return SimpleGraph(one(T))
if Graphs.isbounded(k) && (BigInt(z)^k - 1) ÷ (z - 1) > typemax(T)
throw(InexactError(:convert, T, (BigInt(z)^k - 1) ÷ (z - 1)))
end

n = T((z^k - 1) / (z - 1))
Copy link
Member

Choose a reason for hiding this comment

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

You already calculate the number of vertices with (BigInt(z)^k - 1) ÷ (z - 1), so we we could just reuse this for n.

With the expression n = T((z^k - 1) / (z - 1)) there are two issues:

  • z^k might overflow before converting it to T
  • By using the / operator instead of ÷ your intermediate result will be a double

Copy link
Member

Choose a reason for hiding this comment

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

A lot of these overflow issues probably won't happen if we restrict k and z to Int in the function definition.

ne = n - 1
fadjlist = Vector{Vector{T}}(undef, n)
@inbounds fadjlist[1] = convert.(T, 2:(z + 1))
@inbounds for l in 2:(k - 1)
w = (z^(l - 1) - 1) ÷ (z - 1)
x = w + z^(l - 1)
@simd for i in 1:(z^(l - 1))
j = w + i
fadjlist[j] = [
T(ceil((j - x) / z) + w)
convert.(T, (x + (i - 1) * z + 1):(x + i * z))
]
end
end
l = k
w = (z^(l - 1) - 1) ÷ (z - 1)
x = w + z^(l - 1)
@inbounds @simd for j in (w + 1):x
fadjlist[j] = T[ceil((j - x) / z) + w]
end
return SimpleGraph(ne, fadjlist)
end

Copy link
Member

Choose a reason for hiding this comment

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

I would add the docstring also to this function.

Suggested change
"""
regular_tree(k, z)
Create a regular tree or [perfect z-ary tree](https://en.wikipedia.org/wiki/M-ary_tree#Types_of_m-ary_trees):
a `k`-level tree where all nodes except the leaves have exactly `z` children.
For `z = 2` one recovers a binary tree.
# Examples
```jldoctest
julia> regular_tree(4, 3)
{40, 39} undirected simple Int64 graph
julia> regular_tree(5, 2) == binary_tree(5)
true
```
"""

Copy link
Member

Choose a reason for hiding this comment

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

It's quite often the case, that the docstrings for multiple methods of a function are only added for a single method - I think that would also be better here, than adding docstrings for both methods of this function.

regular_tree(k::Integer, z::Integer) = regular_tree(Int64, k, z)

"""
roach_graph(k)

Expand Down
17 changes: 17 additions & 0 deletions test/simplegraphs/generators/staticgraphs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,23 @@
@test isvalid_simplegraph(g)
end

@testset "Regular Trees" begin
g = @inferred(regular_tree(3, 3))
I = [1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]
J = [2, 3, 4, 1, 5, 6, 7, 8, 1, 9, 10, 1, 11, 12, 13, 2, 2, 2, 3, 3, 3, 4, 4, 4]
V = ones(Int, length(I))
Adj = sparse(I, J, V)
@test Adj == sparse(g)
@test isvalid_simplegraph(g)
@test_throws InexactError regular_tree(Int8, 4, 5)
g = @inferred(regular_tree(Int16, 4, 5))
@test isvalid_simplegraph(g)
# test that z = 1 recovers a path graph
@test all(regular_tree(k, 1) == path_graph(k) for k in 0:10)
# test that z = 2 recovers a binary tree
@test all(regular_tree(k, 2) == binary_tree(k) for k in 0:10)
end

@testset "Roach Graphs" begin
rg3 = @inferred(roach_graph(3))
# [3]
Expand Down