Skip to content
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