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

Prufer coding for trees #206

Merged
merged 28 commits into from
Jan 6, 2023
Merged

Conversation

SimonCoste
Copy link
Contributor

@SimonCoste SimonCoste commented Dec 31, 2022

Hi,
I added elementary functionalities related to trees. There are 3 exported functions,

  • is_tree checks if a simple graph is a tree
  • prufer_encode creates the Prüfer sequence associated to a tree
  • prufer_decode creates the tree associated with a Prüfer sequence

This is my first PR; I did my best at following the style guide and provide a doc, please do not hesitate to explain how things should be done.

I also modified the docs but I did not succeed in having it correctly deployed locally; I get a new trees pages but none of the docstrings I wrote seem to be parsed by the Documenter. I also get the following possibly related warning:

Warning: Documenter could not auto-detect the building environment Skipping deployment.
└ @ Documenter ~/.julia/packages/Documenter/bFHi4/src/deployconfig.jl:75

Related: https://github.com/JuliaGraphs/Graphs.jl/pull/194. I'll add a random Cayley tree generator very soon.

@codecov
Copy link

codecov bot commented Jan 1, 2023

Codecov Report

Merging #206 (426d438) into master (897e183) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #206      +/-   ##
==========================================
+ Coverage   97.40%   97.42%   +0.02%     
==========================================
  Files         109      113       +4     
  Lines        6470     6531      +61     
==========================================
+ Hits         6302     6363      +61     
  Misses        168      168              

@simonschoelly simonschoelly added the enhancement New feature or request label Jan 1, 2023
@simonschoelly
Copy link
Member

Hi, I already added a few comments so far - I will add more later, but I need to figure out how these Prüfer sequences work first.

SimonCoste and others added 2 commits January 2, 2023 21:15
Co-authored-by: Simon Schölly <[email protected]>
Co-authored-by: Simon Schölly <[email protected]>
Co-authored-by: Simon Schölly <[email protected]>
@simonschoelly
Copy link
Member

A few more minor things (and one major) - appart from that I haven't worked with the documentation for a long time, so I have not found out what is wrong here, maybe @gdalle knows more about that, otherwise I will try too look at it in the next fews days.

@SimonCoste
Copy link
Contributor Author

SimonCoste commented Jan 4, 2023

Hi, I adapted to all your remarks.

Importantly, I also added the uniform_tree(n) function. It generates a uniform tree over the $n^{n-2}$ such trees by decoding a random uniform Prüfer sequence.

Edit: I also corrected the docs, you were right about the space after ```: if you forget it, the function isnt parsed by Documenter. The doc looks fine now.

@SimonCoste
Copy link
Contributor Author

I fixed a small bug (must specify seed=nothing in rng_from_rng_or_seed arguments). Everything should be fine now !

@simonschoelly
Copy link
Member

The documentation is also working now, right? Then it does indeed seem ready to be merged.

@SimonCoste
Copy link
Contributor Author

Yes, the doc is properly working!

@simonschoelly simonschoelly merged commit 1a7594a into JuliaGraphs:master Jan 6, 2023
@simonschoelly
Copy link
Member

Great, thank you - for your next PR, the tests should also run automatically, its just for someones first PR, that we have to to trigger the tests automatically

@SimonCoste
Copy link
Contributor Author

SimonCoste commented Jan 6, 2023

In the near future (~2 months) I'll add more random tree generators, possibly: 

  • random binary trees
  • random recursive trees
  • random uniform trees with a fixed degree sequence
  • random uniform trees with a fixed number of leaves.

Thanks Simon for the support !

@simonschoelly
Copy link
Member

In the near future (~2 months) I'll add more random tree generators, possibly: 

* random binary trees

* random recursive trees

* random uniform trees with a fixed degree sequence

* random uniform trees with a fixed number of edges.

Thanks Simon for the support !

That are definitely welcome additions. When taking inspiration from existing code for random graphs here, you just have to be a bit careful, that code sometimes quite old and buggy, and might not even calculate the expected distribution (If I remember correctly, for random_regular_ graph).

@SimonCoste
Copy link
Contributor Author

SimonCoste commented Jan 14, 2023

What is this potential bias in random_regular_graph ? I'll have a look.

@simonschoelly
Copy link
Member

I don't know where exactly the error is, but lets look at an example, the 2-regular graphs on 6-vertices, assuming that the vertices are labeled:

There 60 connected graphs (a single cycle) given by 5!/2 = 60.
There are 10 disconnected graphs (two cycles of 3 vertices each) given by {5 choose 2} = 10.

Therefore the probability of getting a connected graph should be 60 / (10 + 60) = 6/7 ≈ 0.857.

We can also estimate this probability by sampling repeatedly with random_regular_graph:

julia> using Graphs

julia> p_connected(n) = count(is_connected(random_regular_graph(6, 2)) for _ in 1:n) / n
p_connected (generic function with 1 method)

julia> p_connected(10^6)
0.6911

so there is something off with that algorithm And it can also not be for the (probably much more difficult) problem of random regular graphs on unlabeled vertices, because then the probability would be 1/2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants