Add regular tree generator #197

stecrotti · 2022-11-23T15:25:54Z

Generator for a regular tree, also known as Bethe Lattice or Cayley Tree, which generalizes a binary tree to arbitrary degree.

I tried to keep the code as close as possible to the one for binary_tree.

codecov · 2022-11-24T07:41:33Z

Codecov Report

Merging #197 (b26cf91) into master (ea6bcfe) will decrease coverage by 0.13%.
The diff coverage is 92.85%.

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
- Coverage   97.40%   97.26%   -0.14%     
==========================================
  Files         109      109              
  Lines        6468     6506      +38     
==========================================
+ Hits         6300     6328      +28     
- Misses        168      178      +10

src/SimpleGraphs/generators/staticgraphs.jl

simonschoelly · 2022-11-24T16:02:31Z

src/SimpleGraphs/generators/staticgraphs.jl

+    k <= 0 && return SimpleGraph(0)
+    k == 1 && return SimpleGraph(1)


In these cases we return a graph of of eltype Int, but in all other cases we return a graph of type T.

To be completely honest, I am not sure if it is the best idea to determine the eltype of the graphs from k and z - we do that indeed for other graphs here, but I never really liked that.

Maybe we can have a function signature

regular_tree(T::Type{<:Integer}, k::Integer, z::Integer)

and then define something like

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

that for be similar to the function sprand from SparseArrays for example.

simonschoelly · 2022-11-24T16:11:31Z

src/SimpleGraphs/generators/staticgraphs.jl

+"""
+    regular_tree(k::Integer, z::integer)
+
+Create a [regular tree](https://en.wikipedia.org/wiki/Bethe_lattice), 


That wikipedia article might be a bit confusing, because it talks about infinite trees in the introduction.

Maybe we can decribe that a bit more clearly here?

If I understood correctly, what you want to create is a perfect k-ary tree such as described here? https://en.wikipedia.org/wiki/M-ary_tree#Types_of_m-ary_trees

stecrotti · 2022-11-24T23:01:25Z

By the way I called it 'regular tree' because of the analogy with regular graphs but that might not be too accurate because:

A regular tree is not a regular graph (because of the leaves)
z-regular graphs have nodes of degree z while z-ary trees have nodes of degree z+1, potentially confusing

That said, mary_tree(k, m), zary_tree(k, z) don't look great to me.

stecrotti · 2022-11-30T09:47:08Z

Changes:

New method with type regular_tree(T::Type{<:Integer}, k::Integer, z::Integer)
Improved docs with correct wiki page
InexactError instead of DomainError when number of vertices is not representable
Assert z > 0, fallback to a path graph in case z = 1
Appropriate conversions to the provided type T, checked by test

simonschoelly · 2022-11-30T11:01:27Z

By the way I called it 'regular tree' because of the analogy with regular graphs but that might not be too accurate because:
1. A regular tree is not a regular graph (because of the leaves)

2. z-regular graphs have nodes of degree z while z-ary trees have nodes of degree z+1, potentially confusing
That said, mary_tree(k, m), zary_tree(k, z) don't look great to me.

I think regular tree is perfectly fine. For example here, the same term is used: https://www.wolframalpha.com/examples/mathematics/discrete-mathematics/graph-theory/regular-trees/

aurorarossi

Some comments on the docstrings. If you agree with the style then I will open a PR to fix double_binary_tree and binary_tree

aurorarossi · 2022-12-07T13:47:29Z

src/SimpleGraphs/generators/staticgraphs.jl

@@ -536,6 +536,60 @@ function double_binary_tree(k::Integer)
    return g
 end

+"""
+    regular_tree([T=Int64, ], k::Integer, z::integer)


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)

aurorarossi · 2022-12-07T13:59:08Z

src/SimpleGraphs/generators/staticgraphs.jl

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


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

aurorarossi · 2022-12-07T13:59:15Z

src/SimpleGraphs/generators/staticgraphs.jl

+
+julia> regular_tree(5, 2) == binary_tree(5)
+true


Same as above.

Suggested change

julia> regular_tree(5, 2) == binary_tree(5)

true

aurorarossi · 2022-12-07T14:01:58Z

src/SimpleGraphs/generators/staticgraphs.jl

+    end
+    return SimpleGraph(ne, fadjlist)
+end
+


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

```

"""

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.

dapias · 2022-12-14T08:33:30Z

I just realized that a $k$-ary tree is not the same as a Cayley tree of connectivity $k$ because in a $k$-ary tree, the root has connectivity $k$, and all the nodes excepting those in the leaves have connectivity $k+1$. Look, for instance, at the attached plot, nodes 2,3 and 4 have connectivity four and not three as it would be the case for a Cayley tree

stecrotti · 2022-12-14T12:47:42Z

Agree! Indeed it was corrected and should be ok in the current version of this PR

dapias · 2022-12-15T07:35:38Z

Is there a function to create an actual Cayley tree?

stecrotti · 2022-12-15T14:44:39Z

Not that i know of

dapias · 2022-12-16T09:42:56Z

I am not an expert, but it seems trivial to be implemented, given the actual code for the tree. After setting up the central node (root), one only needs to demand that each child has $(z-1)$ children instead of $z$

simonschoelly · 2022-12-18T23:17:56Z

src/SimpleGraphs/generators/staticgraphs.jl

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


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

simonschoelly · 2022-12-18T23:25:44Z

src/SimpleGraphs/generators/staticgraphs.jl

+    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))


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

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

stecrotti · 2022-12-19T10:25:49Z

Changed the signature to match the rest of the file as suggested by @aurorarossi, but kept only one docstring.

Fixed potential overflow as suggested by @simonschoelly.

simonschoelly · 2022-12-20T00:31:55Z

There is this one comment from me, about k >= 0 - not sure if you have seen it, but feel free to ignore that it you think we should not change anything there.

Otherwise looks good for me - I did not check the math, but choose to believe the tests.

stecrotti · 2022-12-20T09:06:57Z

Sure, i replied under your review comment:

I stuck to what was used in the rest of the file which is that for k<=0 it returns a SimpleGraph(zero(T)). I guess there was a good reason for that? If not it makes total sense to error if k<0.

stecrotti added 4 commits November 23, 2022 13:53

add regular tree generator with test

01896fe

docstring for regular trees, one more test

dac8f9a

formatted according to JuliaFormatter

7abb310

fix error in docstring

9a0fb29

stecrotti marked this pull request as ready for review November 23, 2022 15:28

add test for domain error

f441197

stecrotti marked this pull request as draft November 24, 2022 12:52

aurorarossi reviewed Nov 24, 2022

View reviewed changes

src/SimpleGraphs/generators/staticgraphs.jl Outdated Show resolved Hide resolved

simonschoelly reviewed Nov 24, 2022

View reviewed changes

new signature, InexactError, works for Int types

ec15727

stecrotti marked this pull request as ready for review November 30, 2022 09:47

stecrotti requested a review from aurorarossi November 30, 2022 21:43

aurorarossi reviewed Dec 7, 2022

View reviewed changes

simonschoelly reviewed Dec 18, 2022

View reviewed changes

uniform signature style, fix overflow

b26cf91

simonschoelly mentioned this pull request Jan 1, 2023

Prufer coding for trees #206

Merged

gdalle added the enhancement New feature or request label Jun 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add regular tree generator #197

Add regular tree generator #197

stecrotti commented Nov 23, 2022 •

edited

codecov bot commented Nov 24, 2022 •

edited

simonschoelly Nov 24, 2022

simonschoelly Nov 24, 2022

simonschoelly Nov 24, 2022

simonschoelly Nov 24, 2022

stecrotti commented Nov 24, 2022

stecrotti commented Nov 30, 2022

simonschoelly commented Nov 30, 2022

aurorarossi left a comment

aurorarossi Dec 7, 2022

aurorarossi Dec 7, 2022

aurorarossi Dec 7, 2022

aurorarossi Dec 7, 2022

simonschoelly Dec 8, 2022

dapias commented Dec 14, 2022

stecrotti commented Dec 14, 2022

dapias commented Dec 15, 2022

stecrotti commented Dec 15, 2022

dapias commented Dec 16, 2022 •

edited

simonschoelly Dec 18, 2022

simonschoelly Dec 18, 2022

simonschoelly Dec 18, 2022

stecrotti commented Dec 19, 2022

simonschoelly commented Dec 20, 2022

stecrotti commented Dec 20, 2022

		k <= 0 && return SimpleGraph(0)
		k == 1 && return SimpleGraph(1)

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

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

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

Add regular tree generator #197

Are you sure you want to change the base?

Add regular tree generator #197

Conversation

stecrotti commented Nov 23, 2022 • edited

codecov bot commented Nov 24, 2022 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stecrotti commented Nov 24, 2022

stecrotti commented Nov 30, 2022

simonschoelly commented Nov 30, 2022

aurorarossi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dapias commented Dec 14, 2022

stecrotti commented Dec 14, 2022

dapias commented Dec 15, 2022

stecrotti commented Dec 15, 2022

dapias commented Dec 16, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stecrotti commented Dec 19, 2022

simonschoelly commented Dec 20, 2022

stecrotti commented Dec 20, 2022

stecrotti commented Nov 23, 2022 •

edited

codecov bot commented Nov 24, 2022 •

edited

dapias commented Dec 16, 2022 •

edited