implemented Erdos-Renyi model generation #2253
Conversation
|
|
|
Apologies if I did anything wrong. This is my first PR. |
miratepuffin
left a comment
miratepuffin
left a comment
There was a problem hiding this comment.
Couple innitial thoughts
raphtory/src/graphgen/erdos_renyi.rs
Outdated
| use crate::prelude::*; | ||
|
|
||
| #[test] | ||
| fn test_erdos_renyi_small_graph() { |
There was a problem hiding this comment.
Wanna add some tests where you add 100,000 nodes or somethign else reasonably big.
Also a good idea to look at adding some tests for graphs which already have nodes and edges present.
raphtory/src/graphgen/erdos_renyi.rs
Outdated
| for other_id in all_ids.iter() { | ||
| if id != other_id && rng.gen::<f64>() < p { | ||
| latest_time += 1; | ||
| graph |
There was a problem hiding this comment.
Do we consider that an edge may already be present?
There was a problem hiding this comment.
Could I delete an edge if its present?
raphtory/src/graphgen/erdos_renyi.rs
Outdated
| #[test] | ||
| fn test_erdos_renyi_small_graph() { | ||
| let graph = Graph::new(); | ||
| let n_nodes = 5; |
There was a problem hiding this comment.
Could also add some tests to see if the graphs have expected stats e.g. density
There was a problem hiding this comment.
like a hypothesis test?
|
You might also be interested in this paper: https://kim246.wwwdns.kim.uni-konstanz.de/publications/bb-eglrn-05.pdf In section IIa they have an algorithm for generating these graphs without looping over all pairs of nodes which is going to be a lot more efficient for sparse graphs. Note that you can parallelise it by dividing the edges into chunks as the geometric distribution is memory-free. |
|
appreciate it |
|
@LJeub Would what you suggested only be like an extra performant feature that a user can specify? |
DanielLacina
force-pushed
the
erdos_renyl
branch
from
a2bcc3d to
f210713
Compare
This is an optimisation that should almost always be faster in realistic scenarios (it reduces the number of random variables generated to O(m) where m is the actual number of edges in the final graph rather than O(n^2)). However, you do not need to implement it for this first version. We can change it later and also benchmark the two implementations. Just thought you might be interested... |
ljeub-pometry
left a comment
ljeub-pometry
left a comment
There was a problem hiding this comment.
- need to fix the error handling to actually return errors
- probably better if this doesn't take a graph as input
ljeub-pometry
left a comment
ljeub-pometry
left a comment
There was a problem hiding this comment.
You are basically there, just need to add some ? to actually return the errors.
ljeub-pometry
left a comment
ljeub-pometry
left a comment
There was a problem hiding this comment.
this should be the last change before merging it 🤞
ljeub-pometry
left a comment
ljeub-pometry
left a comment
There was a problem hiding this comment.
This looks ok for a first version. We need to expose this in python as well.
ljeub-pometry
changed the title
ljeub-pometry
changed the title
4 participants
No description provided.