-
Notifications
You must be signed in to change notification settings - Fork 66
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
Random graph methods #1039
base: master
Are you sure you want to change the base?
Random graph methods #1039
Conversation
* - YYYY/MM Author: Description of the modification | ||
*/ | ||
|
||
#ifndef RANDOM__H_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we put GUDHI in the name of the macro, to reduce the risk of conflicts?
} | ||
|
||
private: | ||
std::mt19937 gen_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we pick one, mt19937_64 may be better, the platforms we target are all 64 bits (and then the seed is 64 bits as well).
#include <cstddef> // for std::size_t | ||
|
||
namespace Gudhi { | ||
std::random_device rd; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a global object, it may cause trouble if you try to link 2 .o that both use gudhi.
You could mark it inline
(and then also hide it as a static member of the class or in some subnamespace), or create a temporary one just where it is used.
#include <iostream> | ||
#endif // DEBUG_TRACES | ||
|
||
std::random_device rd; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Argh!
std::fill(to_permute.begin(), to_permute.begin() + 2, true); | ||
|
||
std::size_t nb_permutations = (nb_vertices * (nb_vertices - 1)) / 2; | ||
auto random_values = Gudhi::Random().get_range<double>(nb_permutations); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are going to create Random objects left and right, I don't see how this will help determinism. There is no way for the user to specify a seed...
std::vector<Vertex_handle> vertices(nb_vertices); | ||
std::iota(vertices.begin(), vertices.end(), 0); // vertices is { 0, 1, 2, ..., 99 } when nb_vertices is 100 | ||
st.insert_batch_vertices(vertices); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::vector<Vertex_handle> vertices(nb_vertices); | |
std::iota(vertices.begin(), vertices.end(), 0); // vertices is { 0, 1, 2, ..., 99 } when nb_vertices is 100 | |
st.insert_batch_vertices(vertices); | |
st.insert_batch_vertices(boost::irange(nb_vertices)); |
https://www.boost.org/doc/libs/1_85_0/libs/range/doc/html/range/reference/ranges/irange.html (or counting_range)
} | ||
|
||
template<typename Simplex_tree> | ||
void simplex_tree_random_flag_complex( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't produce a flag complex, it produces a graph.
namespace Gudhi { | ||
|
||
template <typename Vertex_handle> | ||
std::vector<std::array<Vertex_handle, 2>> random_edges(Vertex_handle nb_vertices, double density = 0.15) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The algorithm in there seems very complicated. What do you want exactly?
- For each pair of vertices, add the edge with proba 15% (independently)
- Get exactly 15% * n(n-1)/2 edges, chosen randomly
- Other
A new helper class
Gudhi::Random
that can be constructed with a user given seed, or from a random seed if not given.This could help for #795 resolution.
The use of
Gudhi::Random
everywherestd::mt19937
is used in legacy code is not part of this PR, but can be done after on a different PR.The purpose of this PR is to provide 2 new methods to construct random graph:
random_edges
that returns X% (cf.density argument) of permuted vertices (cf. nb_vertices argument), seen as a list of edgessimplex_tree_random_flag_complex
that fills a Simplex_tree withrandom_edges
, but also with random filtration valuesMaybe
simplex_tree_random_flag_complex
is a bit slow:But this is not what we want to benchmark.