Fix directed gnp_random_graph (#954)

* Initial tests

* Convert gnp_random to networkx version

* Fix directed gnp random graph

* Format

* Fix test

* Fix dot tests

* Expand directed gnp test

* Lint

* Remove ddt

* Fix ret-back-compat

* Release note

* Ret tests

* Lint

* Ret again

* Line too long

* Add try_into for usize to isize

* Use match for isize Err

releasenotes/notes/fix-gnp-random-directed-41f2a436c49b9064.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixed an issue where the :func:`~rustworkx.generators.directed_gnp_random_graph`
+    and the :func:`~rustworkx-core.generators.gnp_random_graph` for directed graphs
+    produced a graph where lower node numbers had only a small number of edges
+    compared to what was expected.

rustworkx-core/src/generators/random_graph.rs

@@ -105,6 +105,7 @@ where
     if !(0.0..=1.0).contains(&probability) {
         return Err(InvalidInputError {});
     }
+
     if probability > 0.0 {
         if (probability - 1.0).abs() < std::f64::EPSILON {
             for u in 0..num_nodes {
@@ -119,32 +120,49 @@ where
                 }
             }
         } else {
-            let mut v: isize = if directed { 0 } else { 1 };
-            let mut w: isize = -1;
-            let num_nodes: isize = num_nodes as isize;
+            let num_nodes: isize = match num_nodes.try_into() {
+                Ok(nodes) => nodes,
+                Err(_) => return Err(InvalidInputError {}),
+            };
             let lp: f64 = (1.0 - probability).ln();
-
             let between = Uniform::new(0.0, 1.0);
-            while v < num_nodes {
-                let random: f64 = between.sample(&mut rng);
-                let lr: f64 = (1.0 - random).ln();
-                let ratio: isize = (lr / lp) as isize;
-                w = w + 1 + ratio;
 
-                if directed {
-                    // avoid self loops
+            // For directed, create inward edges to a v
+            if directed {
+                let mut v: isize = 0;
+                let mut w: isize = -1;
+                while v < num_nodes {
+                    let random: f64 = between.sample(&mut rng);
+                    let lr: f64 = (1.0 - random).ln();
+                    w = w + 1 + ((lr / lp) as isize);
+                    while w >= v && v < num_nodes {
+                        w -= v;
+                        v += 1;
+                    }
+                    // Skip self-loops
                     if v == w {
-                        w += 1;
+                        w -= v;
+                        v += 1;
+                    }
+                    if v < num_nodes {
+                        let v_index = graph.from_index(v as usize);
+                        let w_index = graph.from_index(w as usize);
+                        graph.add_edge(w_index, v_index, default_edge_weight());
                     }
                 }
-                while v < num_nodes && ((directed && num_nodes <= w) || (!directed && v <= w)) {
+            }
+
+            // For directed and undirected, create outward edges from a v
+            // Nodes in graph are from 0,n-1 (start with v as the second node index).
+            let mut v: isize = 1;
+            let mut w: isize = -1;
+            while v < num_nodes {
+                let random: f64 = between.sample(&mut rng);
+                let lr: f64 = (1.0 - random).ln();
+                w = w + 1 + ((lr / lp) as isize);
+                while w >= v && v < num_nodes {
                     w -= v;
                     v += 1;
-                    // avoid self loops
-                    if directed && v == w {
-                        w -= v;
-                        v += 1;
-                    }
                 }
                 if v < num_nodes {
                     let v_index = graph.from_index(v as usize);
@@ -533,7 +551,7 @@ mod tests {
         let g: petgraph::graph::DiGraph<(), ()> =
             gnp_random_graph(20, 0.5, Some(10), || (), || ()).unwrap();
         assert_eq!(g.node_count(), 20);
-        assert_eq!(g.edge_count(), 104);
+        assert_eq!(g.edge_count(), 189);
     }
 
     #[test]

tests/rustworkx_tests/digraph/test_dot.py

@@ -57,17 +57,24 @@ def test_digraph_to_dot_to_file(self):
     def test_digraph_empty_dicts(self):
         graph = rustworkx.directed_gnp_random_graph(3, 0.9, seed=42)
         dot_str = graph.to_dot(lambda _: {}, lambda _: {})
-        self.assertEqual("digraph {\n0 ;\n1 ;\n2 ;\n0 -> 1 ;\n0 -> 2 ;\n}\n", dot_str)
+        self.assertEqual(
+            "digraph {\n0 ;\n1 ;\n2 ;\n0 -> 1 ;\n0 -> 2 ;\n1 -> 2 ;\n2 -> 0 ;\n2 -> 1 ;\n}\n",
+            dot_str,
+        )
 
     def test_digraph_graph_attrs(self):
         graph = rustworkx.directed_gnp_random_graph(3, 0.9, seed=42)
         dot_str = graph.to_dot(lambda _: {}, lambda _: {}, {"bgcolor": "red"})
         self.assertEqual(
-            "digraph {\nbgcolor=red ;\n0 ;\n1 ;\n2 ;\n0 -> 1 ;\n" "0 -> 2 ;\n}\n",
+            "digraph {\nbgcolor=red ;\n0 ;\n1 ;\n2 ;\n0 -> 1 \
+;\n0 -> 2 ;\n1 -> 2 ;\n2 -> 0 ;\n2 -> 1 ;\n}\n",
             dot_str,
         )
 
     def test_digraph_no_args(self):
         graph = rustworkx.directed_gnp_random_graph(3, 0.95, seed=24)
         dot_str = graph.to_dot()
-        self.assertEqual("digraph {\n0 ;\n1 ;\n2 ;\n0 -> 1 ;\n0 -> 2 ;\n}\n", dot_str)
+        self.assertEqual(
+            "digraph {\n0 ;\n1 ;\n2 ;\n0 -> 2 ;\n1 -> 2 ;\n1 -> 0 ;\n2 -> 0 ;\n2 -> 1 ;\n}\n",
+            dot_str,
+        )

tests/rustworkx_tests/test_random.py

@@ -17,10 +17,20 @@
 
 
 class TestGNPRandomGraph(unittest.TestCase):
-    def test_random_gnp_directed(self):
+    def test_random_gnp_directed_1(self):
+        graph = rustworkx.directed_gnp_random_graph(15, 0.7, seed=20)
+        self.assertEqual(len(graph), 15)
+        self.assertEqual(len(graph.edges()), 156)
+
+    def test_random_gnp_directed_2(self):
         graph = rustworkx.directed_gnp_random_graph(20, 0.5, seed=10)
         self.assertEqual(len(graph), 20)
-        self.assertEqual(len(graph.edges()), 104)
+        self.assertEqual(len(graph.edges()), 189)
+
+    def test_random_gnp_directed_3(self):
+        graph = rustworkx.directed_gnp_random_graph(22, 0.2, seed=6)
+        self.assertEqual(len(graph), 22)
+        self.assertEqual(len(graph.edges()), 91)
 
     def test_random_gnp_directed_empty_graph(self):
         graph = rustworkx.directed_gnp_random_graph(20, 0)